npawar commented on a change in pull request #5642:
URL: https://github.com/apache/incubator-pinot/pull/5642#discussion_r450461863
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/util/SchemaUtils.java
##########
@@ -39,65 +48,130 @@
private static final Logger LOGGER =
LoggerFactory.getLogger(SchemaUtils.class);
+ private static final SqlAbstractParserImpl.Metadata SQL_PARSER_METADATA =
SqlParser.create("",
+
SqlParser.configBuilder().setConformance(SqlConformanceEnum.BABEL).setParserFactory(SqlBabelParserImpl.FACTORY)
+ .build()).getMetadata();
+
/**
- * Validates that for a field spec with transform function, the source
column name and destination column name are exclusive
- * i.e. do not allow using source column name for destination column
+ * Validates the following:
+ * 1) Checks if sql reserved keywords are being used as field names. This
check can be disabled, for not breaking existing schemas with invalid names
+ * 2) Checks valid transform function -
+ * for a field spec with transform function, the source column name and
destination column name are exclusive i.e. do not allow using source column
name for destination column
+ * ensure transform function string can be used to create a {@link
FunctionEvaluator}
+ * 3) Checks valid timeFieldSpec - if incoming and outgoing granularity spec
are different a) the names cannot be same b) cannot use SIMPLE_DATE_FORMAT for
conversion
+ * 4) Checks valid dateTimeFieldSpecs - checks format and granularity string
+ * 5) Schema validations from {@link Schema#validate(Logger)}
*/
public static boolean validate(Schema schema) {
- return validate(schema, LOGGER);
+ return validate(schema, true, LOGGER);
}
/**
* Validates the following:
- * 1) for a field spec with transform function, the source column name and
destination column name are exclusive
- * i.e. do not allow using source column name for destination column
- * 2) Basic schema validations
+ * 1) Checks if sql reserved keywords are being used as field names. This
check can be disabled, for not breaking existing schemas with invalid names
+ * 2) Checks valid transform function -
+ * for a field spec with transform function, the source column name and
destination column name are exclusive i.e. do not allow using source column
name for destination column
+ * ensure transform function string can be used to create a {@link
FunctionEvaluator}
+ * 3) Checks valid timeFieldSpec - if incoming and outgoing granularity spec
are different a) the names cannot be same b) cannot use SIMPLE_DATE_FORMAT for
conversion
+ * 4) Checks valid dateTimeFieldSpecs - checks format and granularity string
+ * 5) Schema validations from {@link Schema#validate(Logger)}
+ *
+ * @param validateFieldNames if false, does not validate field names. This
is to prevent validation failing on existing schemas with invalid field names
during a schema update
*/
- public static boolean validate(Schema schema, Logger logger) {
+ public static boolean validate(Schema schema, boolean validateFieldNames,
@Nullable Logger logger) {
try {
for (FieldSpec fieldSpec : schema.getAllFieldSpecs()) {
if (!fieldSpec.isVirtualColumn()) {
- String column = fieldSpec.getName();
- String transformFunction = fieldSpec.getTransformFunction();
- if (transformFunction != null) {
- FunctionEvaluator functionEvaluator =
FunctionEvaluatorFactory.getExpressionEvaluator(fieldSpec);
- if (functionEvaluator != null) {
- List<String> arguments = functionEvaluator.getArguments();
- // output column used as input
- if (arguments.contains(column)) {
- logger.error("The arguments of transform function: {}, should
not contain the destination column: {}",
- transformFunction, column);
- return false;
- }
+ if (validateFieldNames && !isValidFieldName(fieldSpec)) {
+ return false;
+ }
+ if (!isValidTransformFunction(fieldSpec)) {
+ return false;
+ }
+ FieldSpec.FieldType fieldType = fieldSpec.getFieldType();
+ if (fieldType.equals(FieldSpec.FieldType.DATE_TIME)) {
+ if (!isValidDateTimeFieldSpec(fieldSpec)) {
+ return false;
}
- } else if
(fieldSpec.getFieldType().equals(FieldSpec.FieldType.TIME)) {
- TimeFieldSpec timeFieldSpec = (TimeFieldSpec) fieldSpec;
- TimeGranularitySpec incomingGranularitySpec =
timeFieldSpec.getIncomingGranularitySpec();
- TimeGranularitySpec outgoingGranularitySpec =
timeFieldSpec.getOutgoingGranularitySpec();
-
- if (!incomingGranularitySpec.equals(outgoingGranularitySpec)) {
- // different incoming and outgoing spec, but same name
- if
(incomingGranularitySpec.getName().equals(outgoingGranularitySpec.getName())) {
- logger.error("Cannot convert from incoming field spec:{} to
outgoing field spec:{} if name is the same",
- incomingGranularitySpec, outgoingGranularitySpec);
- return false;
- } else {
- if
(!incomingGranularitySpec.getTimeFormat().equals(TimeGranularitySpec.TimeFormat.EPOCH.toString())
- || !outgoingGranularitySpec.getTimeFormat()
- .equals(TimeGranularitySpec.TimeFormat.EPOCH.toString())) {
- logger.error(
- "When incoming and outgoing specs are different, cannot
perform time conversion for time format other than EPOCH");
- return false;
- }
- }
+ } else if (fieldType.equals(FieldSpec.FieldType.TIME)) {
+ if (!isValidTimeFieldSpec(fieldSpec)) {
+ return false;
}
}
}
}
} catch (Exception e) {
- logger.error("Exception in validating schema {}",
schema.getSchemaName(), e);
+ LOGGER.error("Exception in validating schema {}",
schema.getSchemaName(), e);
return false;
}
return schema.validate(logger);
}
+
+ /**
+ * Checks if any of the keywords which are reserved under the sql parser are
used
+ */
+ private static boolean isValidFieldName(FieldSpec fieldSpec) {
+ String columnName = fieldSpec.getName();
+ if
(SQL_PARSER_METADATA.isReservedWord(columnName.toUpperCase(Locale.ROOT))) {
Review comment:
yes that was unnecessary, removed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
