>From Dmitry Lychagin <[email protected]>: Dmitry Lychagin has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883 )
Change subject: [ASTERIXDB-2980][*DB][IDX] Add the option "CAST (DEFAULT NULL)" to CREATE INDEX statement ...................................................................... Patch Set 2: (6 comments) Also as discussed, we need to check that CAST modifier is not allowed if a field type is known (i.e. from the closed part). https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java@874 PS2, Line 874: } else if (IndexUtil.castDefaultNull(index)) { I'd make IndexUtil.castDefaultNull the first block of the if statement: if (IndexUtil.castDefaultNull(index)) { ... } else if (index.isEnforced()) { ...BuiltinFunctions.CAST_TYPE ... } else { ...BuiltinFunctions.CAST_TYPE_LAX ... } https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java@1289 PS2, Line 1289: "Cast Default Null cannot be specified together with EXCLUDE UNKNOWN KEY"); As we discussed, we need to remove this restriction. (separate change) https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/ViewUtil.java File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/ViewUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/ViewUtil.java@227 PS2, Line 227: public static FunctionIdentifier getTypeConstructorWithFormat(IAType type) { I think you'll need to move getTypeConstructorWithFormat() into TypeUtil as well in the future. Because we'll need to support date/time cast format modifier in CREATE INDEX as well. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj@229 PS2, Line 229: private static final String CAST = "CAST"; I think we should make CAST a keyword. We'll have to do it anyway in the future when we support CAST expressions, so might as well do it now. (could be in a follow up change). https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java@372 PS2, Line 372: public OptionalBoolean isCastDefaultNull() { should we also change this to getCastDefaultNull() to align with CreateIndexStatement? (and probable also change isExcludeUnknownKey() to getExcludeUnknownKey() ) https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entitytupletranslators/IndexTupleTranslator.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entitytupletranslators/IndexTupleTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883/2/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entitytupletranslators/IndexTupleTranslator.java@816 PS2, Line 816: aString.setValue(MetadataRecordTypes.FIELD_NAME_DEFAULT); I think instead of "Default": null, we should have a record field to indicate that the CAST modifier was specified: "Cast": { "Default": null } . In the future we'll be added date/time formats into the "cast" record: "Cast": { "Default": null, "DataFormat": [ .... ] }. (see how ViewDetails writes FIELD_NAME_DATA_FORMAT for more on this) -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13883 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: I3a3ffd3735f1b311bd532dda955e08bf150ced31 Gerrit-Change-Number: 13883 Gerrit-PatchSet: 2 Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Comment-Date: Mon, 01 Nov 2021 19:31:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
