>From Ali Alsuliman <[email protected]>: Attention is currently required from: Calvin Thomas Dani, Ian Maxon, Wail Alkowaileet. Ali Alsuliman has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044 )
Change subject: [NO ISSUE] Added schema inference for row based records. ...................................................................... Patch Set 7: (23 comments) File README.md: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/3d9d32e8_38c34f6b PS7, Line 26: A Can you revert the changes in this file? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/1da00b57_6da217e9 PS7, Line 87: } It looks like your IDE is doing some formatting. Can you revert such changes (empty lines, ... etc)? File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/bc4e6a85_271a97e7 PS7, Line 170: } Could you revert this? File asterixdb/asterix-om/pom.xml: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/e04b3375_a7c5f8e0 PS7, Line 156: <artifactId>log4j-api</artifactId> Any reason for this removal? File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/adm/AOptionalFieldPrinterFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/5e917799_5e402529 PS7, Line 46: private IPrinter stringPrinter; We don't need this since `fieldPrinter` should take care of whatever type the field is. File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/AOptionalFieldPrinterFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/c9d656d5_ebd54219 PS7, Line 46: private IPrinter stringPrinter; Same thing. This is not needed. File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/json/clean/AOptionalFieldPrinterFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/754a088e_76787d70 PS7, Line 45: private IPrinter stringPrinter; Same thing. This is not needed. File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/json/lossless/AOptionalFieldPrinterFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/24afc470_7a1f5473 PS7, Line 45: private IPrinter stringPrinter; Same thing. This is not needed. File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/api/IRowMetadata.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/21fe1a97_41c1e8c9 PS7, Line 39: void abort() throws HyracksDataException; Do you need this method? What's it for? File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/api/IRowWriteMultiPageOp.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/b92155de_efc25bce PS7, Line 32: IRowWriteMultiPageOp There is no implementation for this interface. It should be removed. File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/9cabc234_76b9e4b4 PS7, Line 438: GLOBAL_SCHEM I don't see SCHEMA? check MIN ("agg-min") for example. Also I don't see this added by addFunction() like the SQL versions. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/a62fd86b_a3176a35 PS7, Line 607: intermediate-agg-sql-schema rename it to "agg-intermediate-sql-schema" https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/830cc4bc_20e4b0d9 PS7, Line 633: public static final FunctionIdentifier LOCAL_SQL_SCHEMA = FunctionConstants.newAsterix("agg-local-sql-schema", 1); : public static final FunctionIdentifier GLOBAL_SQL_SCHEMA = FunctionConstants.newAsterix("agg-global-sql-schema", 1); Can you place them next to 'SQL_SCHEMA' and 'INTERMEDIATE_SQL_SCHEMA'? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/6287b854_4d3bc8ee PS7, Line 692: public static final FunctionIdentifier SCALAR_SQL_SCHEMA = FunctionConstants.newAsterix("sql-schema", 1); I don't see an equivalent SCALAR_SCHEMA? check SCALAR_MIN for example. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/8b466809_463c47c5 PS7, Line 704: SERIAL_SQL_SCHEMA SERIAL* functions are used when the GROUP-BY algorithm is a hash group-by. Not all our functions implement SERIAL* versions (implementing AbstractSerializableAggregateFunctionDynamicDescriptor and related classes). If you are not implementing this for hash group-by, then remove these since they are currently not used. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/90e8af82_6ea11a23 PS7, Line 1725: addPrivateFunction(GLOBAL_SQL_SCHEMA, NullableDoubleTypeComputer.INSTANCE, true); Is 'INTERMEDIATE_SQL_SCHEMA' missing? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/4c302289_86f06341 PS7, Line 3130: if (functionInfo.getFunctionIdentifier().getName().contains("array_avg")) { Could you remove this? File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/utils/RowValuesUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/2f4c56b9_dceb2a9f PS7, Line 21: RowValuesUtil We should remove this if not used. File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/utils/RunRowLengthIntArray.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/dd9e933d_35db8bfa PS7, Line 37: RunRowLengthIntArray We should remove this if not used. File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/utils/UnsafeUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/97d3e54d_cf5fc4f0 PS7, Line 29: UnsafeUtil I don't think we need this. We should use our serializers/deserializers. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/schemainferrence/Serialization/fieldNameSerialization.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/7b6cd49f_d3479a3d PS7, Line 34: fieldNameSerialization Remove unused. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/schemainferrence/Serialization/itemNodeSerialization.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/286ed9b9_a75e9831 PS7, Line 33: itemNodeSerialization Remove unused. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/schemainferrence/Serialization/mapSerialization.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044/comment/c1ff8d50_c2dc5219 PS7, Line 35: mapSerialization Remove unused. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19044 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I5550c89d56994996b5021f1cbf0e4f03f8fee45b Gerrit-Change-Number: 19044 Gerrit-PatchSet: 7 Gerrit-Owner: Calvin Thomas Dani <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-Attention: Calvin Thomas Dani <[email protected]> Gerrit-Attention: Ian Maxon <[email protected]> Gerrit-Attention: Wail Alkowaileet <[email protected]> Gerrit-Comment-Date: Wed, 26 Mar 2025 01:07:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
