> On July 29, 2015, 6:39 p.m., Jason Huynh wrote: > > Good job! Had a few questions and formatting nit picks > > > > Not sure but has the build/tests been run on this patch? I would assume > > that there would be a change needed to the sanctionedSerializables.txt as > > there was a new serializable exception added. > > > > Also I think there would need to be a change to sanctionedDataSerializables > > as there have been a few more data serializable additions
True. I have not run the whole build tests yet. I will change it. > On July 29, 2015, 6:39 p.m., Jason Huynh wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledUDAFunction.java, > > line 29 > > <https://reviews.apache.org/r/36906/diff/1/?file=1024325#file1024325line29> > > > > Should we detect if the uda class doesn't exist at this point and throw > > a more descriptive exception? > > > > Otherwise they may just get an npe? Retrieving UDAClass from UDAManager itself will throw UDANotExistsException, so it will never be null, at that point. > On July 29, 2015, 6:39 p.m., Jason Huynh wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/uda/UDADistributionAdvisor.java, > > line 97 > > <https://reviews.apache.org/r/36906/diff/1/?file=1024344#file1024344line97> > > > > Is this meant to just return true and not actually remove from the uda > > manager? Well removeProfile, is usually sent when a node leaves the system. In our case for UDA, there is no such message sent for removal of UDA Profile ( because no profiles are stored , in the first place). > On July 29, 2015, 6:39 p.m., Jason Huynh wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledGroupBySelect.java, > > line 391 > > <https://reviews.apache.org/r/36906/diff/1/?file=1024324#file1024324line391> > > > > what happens if UNDEFINED is accumulated? Where would that be handled? They are handled in each of the in built UDA implementations. For UDA , I suppose User himself will have to check it. I am not sure if we should do the check for the user & not invoke accumulate if the data is undefined .( what if user wants to count the total undefined :-) ?) - Asif ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36906/#review93464 ----------------------------------------------------------- On July 29, 2015, 9:17 a.m., Asif Shahid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36906/ > ----------------------------------------------------------- > > (Updated July 29, 2015, 9:17 a.m.) > > > Review request for geode. > > > Repository: geode > > > Description > ------- > > Support for UserDefinedAggregates in OQL. Support for allowing creation of > UDAs to be used in OQL > > > Implemented changes for uda creation through cache xml ( declarative syntax) > > > Fixed the bug in profile exchange of UDA. Added test for the same > > > Diffs > ----- > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/Aggregator.java > 9f54eba48072466b139151173216238bc180d289 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/QueryService.java > 4a3cb46f4de4f263e8a7c8f26684392346f42e4a > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/UDAExistsException.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledAggregateFunction.java > ba7731026f096cb0bd70d0cc04b391f46492f33a > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledGroupBySelect.java > ad41c309a906267c3383a4d444a62ad83dbd7ca6 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledUDAFunction.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/DefaultQueryService.java > 71d2e86095e51ead86b24c098dbc8db6de8ffa84 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/ProxyQueryService.java > 8a20f10347aee8f3e29b953b5f39baa49dda017a > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QCompiler.java > 4777dc1b9a3b7576180fec5e4dc159ed2b30d882 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/AbstractAggregator.java > 5809df8fe85d691807809898f1753b665f355cc1 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/Avg.java > e1da36d2f57a96829f7f1dc9a71e4ac607d34777 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/AvgBucketNode.java > 96113218cc04c4a98610a1183680fb0da9918c4d > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/AvgDistinct.java > 3863b72a2df3b2e7f27727e409aafcbdae31aa4e > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/AvgDistinctPRQueryNode.java > 2bbc67d2a4b44c60da9882bd95e7b7c289c8fc20 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/AvgPRQueryNode.java > 23654742130dbb1d20f3fb2e45c29eb0dbea46db > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/Count.java > 3026b63e5a3a8f6a3bf1b4553b8e611a111dbd7a > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/CountDistinct.java > 03af70bf6cee6ce0fd37227efbd33b5419a0e6f3 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/CountDistinctPRQueryNode.java > 4e56f303d227ba2f746b8d86b56134c3c3182182 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/CountPRQueryNode.java > 34d34ac17b16bd9b9eb2ca06f022099b798ae191 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/DistinctAggregator.java > 0328d9a4c06a832daf9ca2aabcb2ec8bb22403e4 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/MaxMin.java > 443e62ddeff304824504923cf4e3ddbbc2b6555d > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/Sum.java > c41d0a520bdb7636f9d7c23776149b58679b94e3 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/SumDistinct.java > e4741dc78f03fda61f031cca1668b696466a719e > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/SumDistinctPRQueryNode.java > 99c6fe7cd819ddb9e94bbc85937a79acfccb7f4d > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/uda/UDADistributionAdvisor.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/uda/UDAManager.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/uda/UDAManagerImpl.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/aggregate/uda/UDAMessage.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/parse/ASTAggregateFunc.java > 1bfe010a2109e32a2004ab1e8b12e0971f982254 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/parse/OQLLexer.java > 2ada98da64bff6a504fa921529392dd4bcfd4a9d > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/parse/OQLLexerTokenTypes.java > f30f3c09e5e8fd30211f69dc9a9dce12c87e9c7b > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/parse/OQLLexerTokenTypes.txt > 8c8fda57fc2d40133b88846254f328b0ab6350bb > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/parse/OQLParser.java > bb70b056222e935b98a9c71b155fe6d08738e573 > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/parse/oql.g > e18e480785c6260bcbb00661d2eee1b449e80ae2 > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionAdvisor.java > 75cadc9232ffdbf35197b4b4a6cf6378ce703581 > gemfire-core/src/main/java/com/gemstone/gemfire/internal/DSFIDFactory.java > 3c33553320276ca843f6f80a6ad24043d36293e4 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/DataSerializableFixedID.java > a8a17153b473143fe73ce965bde68ce81a2e0114 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java > 54870002be6d2aadd5232e317661b668fc92be7b > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/InternalCache.java > 7c1fa8eea232baafd7d40c15437ee4f09b20fccf > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/UpdateAttributesProcessor.java > 2ff6f6f20fdc8a590a98701e24ab5597296e9147 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java > b9fcfe7d215c9ff935df52a0d2fa587348e51922 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXml.java > e39663014c9bb4f9d7001c2c9216e24c2e90c490 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXmlGenerator.java > 06eb091a80b09da7718288e67f8622da9387858c > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXmlParser.java > 9f2cbc098a1bf581c9d523603e072ffaf9138abb > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/UDAManagerCreation.java > PRE-CREATION > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/i18n/ParentLocalizedStrings.java > 21fc52ee132f3d72a60587e460be5ea807d92bfa > > gemfire-core/src/main/resources/META-INF/schemas/schema.pivotal.io/gemfire/cache/cache-9.0.xsd > 35af50ccbdbd294282891f1631dbe57b137522c4 > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/GroupByPartitionedQueryDUnitTest.java > d3e1aca82d78ed10bf252687bc8cb511a522d89f > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/UDACreationDUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/UDADUnitImpl.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/UDAPartitionedQueryDUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/UDAPartitionedJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/UDAReplicatedJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/UDATestImpl.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/UDATestInterface.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/CompiledAggregateFunctionJUnitTest.java > 036fbd2370d8edf27202b4c80c53ca8df93c0f91 > > gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/aggregate/AggregatorJUnitTest.java > 47243ad4ce42fa3513ac36adadaef5366f14b103 > > gemfire-core/src/test/java/com/gemstone/gemfire/cache30/CacheXml90DUnitTest.java > b268ada17b2710f69849fca68185d5475521818d > > gemfire-core/src/test/java/com/gemstone/gemfire/cache30/CacheXmlTestCase.java > 8d6bf19ca7954688761353c2e11eb061d2267982 > > Diff: https://reviews.apache.org/r/36906/diff/ > > > Testing > ------- > > > Thanks, > > Asif Shahid > >
