On Tue, Jul 16, 2013 at 1:24 AM, rahul challapalli < [email protected]> wrote:
> Hi Aaron, > > I started looking into the functionality you already added. A few > observations : > > In the Blur.thrift file, AnalyzerDefinition is removed from the > TableDescriptor. Was this intentional? If so can you give us an example of > how to use them? > Removing the AnalyzerDefinition was intentional. The motivation there is to allow the schema (Families,Columns,and Types) to be set/added independently of the creation of the table. I have not created any new thrift rpc calls to add new column definitions but ultimately it will call addColumnDefinition on the FieldManager class. https://git-wip-us.apache.org/repos/asf?p=incubator-blur.git;a=blob;f=blur- query/src/main/java/org/apache/blur/analysis/FieldManager.java;h=2271726e55bb9356ca6f2b6edf7a5fdec46b36c4;hb=ae516a442767b31d2c7e29b07a78aa08ec246dcf > I modified the Blur.thrift(Column and TableDescriptor) and generated the > code. I don't know how to handle scenarios where minor changes are made and > need to be pushed into the branch. Otherwise it becomes a big commit if we > try to associate with a specific JIRA ticket? > I think that you should attach a patch to the jira ticket. I can review and merge then we can work from the same baseline. Then we can repeat that process as many times as needed. > > I added a bunch of code to the MutationHelper class to validate in-bound > columns. Can you check whether my understanding is aligned with the > requirement? > public static Column validateColumn(String family, Column col, > booleanstrict, FieldManager fieldManager) { > > if (strict == true) { > > if (col.type == null) { > > throw new RuntimeException("The type of the column is a required field > for this table. To turn off this behavior set strictTypes=false on the > TableDesciptor"); > > } > > } > > > > FieldTypeDefinition fieldTypeDefinition = > fieldManager.getFieldTypeDefinition(family + "." + col.name); > > if (fieldTypeDefinition == null) { > > // TODO dynamic column : add new column definition > > return col; > > } > > if (!fieldTypeDefinition.getName().equalsIgnoreCase(col.type)) { > > throw new RuntimeException("The type defined in the column does not match > the existing type definition"); > > } > > return col; > > } > Yes this looks good, but just an FYI I like to always throw BlurExceptions instead of RuntimeExceptions. The main reason for this (across the board) is that Thrift will wrap all exceptions that are not BlurExceptions or TExceptions in a TException. When this happens that client thinks that something went wrong with the connection and will retry the call over several times. Thanks! Aaron > > > - Rahul > > > On Tue, Jul 2, 2013 at 4:27 PM, Aaron McCurry <[email protected]> wrote: > > > I have created a new branch where I have been working on rewriting the > > type/analyzer system for what seems like the 3rd or 4th time. So > hopefully > > it will turn out better this time. > > > > > > > https://git-wip-us.apache.org/repos/asf?p=incubator-blur.git;a=shortlog;h=refs/heads/0.2.0-newtypesystem > > > > If you have a chance I would love some feedback on what's been built thus > > far. > > > > > > The o.a.b.analysis package in the blur-query project: > > > > > > > https://git-wip-us.apache.org/repos/asf?p=incubator-blur.git;a=tree;f=blur-query/src/main/java/org/apache/blur/analysis;h=3db57e994d4e60cc81d94641482c69305767fab5;hb=4ebe74ef2e489d8a360220e0d2752c682042ab22 > > > > And the o.a.b.analysis.type package in the blur-query project: > > > > > > > https://git-wip-us.apache.org/repos/asf?p=incubator-blur.git;a=tree;f=blur-query/src/main/java/org/apache/blur/analysis/type;h=44ca6e1114210ffd8d202a29a347f7b77e37142f;hb=4ebe74ef2e489d8a360220e0d2752c682042ab22 > > > > The main classes to start looking at are BaseFileManager and the > > FieldTypeDefinition. They will lead you to several implementations. My > > hope is that this API will allow us to support the given types in Lucene > as > > well as allowing other to create new FieldTypeDefinition(s) and extend > > Blur. > > > > Let me know what you think. Thanks! > > > > Aaron > > >
