You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2022/05/08 14:12:04 UTC
[GitHub] [drill] vvysotskyi opened a new pull request, #2539: DRILL-8216: Use EVF-based JSON reader for Values operator
vvysotskyi opened a new pull request, #2539:
URL: https://github.com/apache/drill/pull/2539
# [DRILL-8216](https://issues.apache.org/jira/browse/DRILL-8216): Use EVF-based JSON reader for Values operator
## Description
The newer Calcite version simplifies and removes casts for literals. It causes wrong results for `drillTypeOf` and `sqlTypeOf` functions since the Values operator uses an old JSON reader which reads integers with bigInt type.
This PR uses EVF-based JSON reader for the Values operator and passes actual data types discovered during planning as the provided schema.
## Documentation
NA
## Testing
Unit tests pass
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] lgtm-com[bot] commented on pull request #2539: DRILL-8216: Use EVF-based JSON reader for Values operator
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on PR #2539:
URL: https://github.com/apache/drill/pull/2539#issuecomment-1120440515
This pull request **fixes 1 alert** when merging d2908869c5c841ac9397995d2e4a65d8ca2ca9cf into 8b498206b35708fa0ae8ef3be3cb41e9d3c13838 - [view on LGTM.com](https://lgtm.com/projects/g/apache/drill/rev/pr-2b7cc30cf73399999872c0e96c054f643707d9f0)
**fixed alerts:**
* 1 for Result of multiplication cast to wider type
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a diff in pull request #2539: DRILL-8216: Use EVF-based JSON reader for Values operator
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on code in PR #2539:
URL: https://github.com/apache/drill/pull/2539#discussion_r870903809
##########
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/scan/v3/schema/SchemaUtils.java:
##########
@@ -234,4 +241,80 @@ static void copyProperties(TupleMetadata source,
dest.setProperty(ScanProjectionParser.PROJECTION_TYPE_PROP, value);
}
}
+
+ /**
+ * Converts specified {@code RelDataType relDataType} into {@link ColumnMetadata}.
+ * For the case when specified relDataType is struct, map with recursively converted children
+ * will be created.
+ *
+ * @param name filed name
+ * @param relDataType filed type
+ * @return {@link ColumnMetadata} which corresponds to specified {@code RelDataType relDataType}
+ */
+ public static ColumnMetadata getColumnMetadata(String name, RelDataType relDataType) {
+ switch (relDataType.getSqlTypeName()) {
+ case ARRAY:
+ return getArrayMetadata(name, relDataType);
+ case MAP:
+ case OTHER:
+ throw new UnsupportedOperationException(String.format("Unsupported data type: %s", relDataType.getSqlTypeName()));
+ default:
+ if (relDataType.isStruct()) {
+ return getStructMetadata(name, relDataType);
+ } else {
+ return new PrimitiveColumnMetadata(
+ MaterializedField.create(name,
+ TypeInferenceUtils.getDrillMajorTypeFromCalciteType(relDataType)));
+ }
+ }
+ }
+
+ /**
+ * Returns {@link ColumnMetadata} instance which corresponds to specified array {@code RelDataType relDataType}.
+ *
+ * @param name name of the filed
+ * @param relDataType the source of type information to construct the schema
+ * @return {@link ColumnMetadata} instance
+ */
+ private static ColumnMetadata getArrayMetadata(String name, RelDataType relDataType) {
+ RelDataType componentType = relDataType.getComponentType();
+ ColumnMetadata childColumnMetadata = getColumnMetadata(name, componentType);
+ switch (componentType.getSqlTypeName()) {
+ case ARRAY:
+ // for the case when nested type is array, it should be placed into repeated list
+ return MetadataUtils.newRepeatedList(name, childColumnMetadata);
+ case MAP:
+ case OTHER:
+ throw new UnsupportedOperationException(String.format("Unsupported data type: %s", relDataType.getSqlTypeName()));
+ default:
+ if (componentType.isStruct()) {
+ // for the case when nested type is struct, it should be placed into repeated map
+ return MetadataUtils.newMapArray(name, childColumnMetadata.tupleSchema());
+ } else {
+ // otherwise creates column metadata with repeated data mode
+ return new PrimitiveColumnMetadata(
+ MaterializedField.create(name,
+ Types.overrideMode(
+ TypeInferenceUtils.getDrillMajorTypeFromCalciteType(componentType),
+ TypeProtos.DataMode.REPEATED)));
+ }
+ }
+ }
+
+ /**
+ * Returns {@link MapColumnMetadata} column metadata created based on specified {@code RelDataType relDataType} with
+ * converted to {@link ColumnMetadata} {@code relDataType}'s children.
+ *
+ * @param name name of the filed
+ * @param relDataType {@link RelDataType} the source of the children for resulting schema
+ * @return {@link MapColumnMetadata} column metadata
+ */
+ private static MapColumnMetadata getStructMetadata(String name, RelDataType relDataType) {
+ TupleMetadata mapSchema = new TupleSchema();
+ relDataType.getFieldList().stream()
+ .map(field -> getColumnMetadata(field.getName(), field.getType()))
Review Comment:
In the code it is called struct. Real map types as they are aren't supported for values operator.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a diff in pull request #2539: DRILL-8216: Use EVF-based JSON reader for Values operator
Posted by GitBox <gi...@apache.org>.
cgivre commented on code in PR #2539:
URL: https://github.com/apache/drill/pull/2539#discussion_r870823130
##########
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/scan/v3/schema/SchemaUtils.java:
##########
@@ -234,4 +241,80 @@ static void copyProperties(TupleMetadata source,
dest.setProperty(ScanProjectionParser.PROJECTION_TYPE_PROP, value);
}
}
+
+ /**
+ * Converts specified {@code RelDataType relDataType} into {@link ColumnMetadata}.
+ * For the case when specified relDataType is struct, map with recursively converted children
+ * will be created.
+ *
+ * @param name filed name
+ * @param relDataType filed type
+ * @return {@link ColumnMetadata} which corresponds to specified {@code RelDataType relDataType}
+ */
+ public static ColumnMetadata getColumnMetadata(String name, RelDataType relDataType) {
+ switch (relDataType.getSqlTypeName()) {
+ case ARRAY:
+ return getArrayMetadata(name, relDataType);
+ case MAP:
+ case OTHER:
+ throw new UnsupportedOperationException(String.format("Unsupported data type: %s", relDataType.getSqlTypeName()));
+ default:
+ if (relDataType.isStruct()) {
+ return getStructMetadata(name, relDataType);
+ } else {
+ return new PrimitiveColumnMetadata(
+ MaterializedField.create(name,
+ TypeInferenceUtils.getDrillMajorTypeFromCalciteType(relDataType)));
+ }
+ }
+ }
+
+ /**
+ * Returns {@link ColumnMetadata} instance which corresponds to specified array {@code RelDataType relDataType}.
+ *
+ * @param name name of the filed
+ * @param relDataType the source of type information to construct the schema
+ * @return {@link ColumnMetadata} instance
+ */
+ private static ColumnMetadata getArrayMetadata(String name, RelDataType relDataType) {
+ RelDataType componentType = relDataType.getComponentType();
+ ColumnMetadata childColumnMetadata = getColumnMetadata(name, componentType);
+ switch (componentType.getSqlTypeName()) {
+ case ARRAY:
+ // for the case when nested type is array, it should be placed into repeated list
+ return MetadataUtils.newRepeatedList(name, childColumnMetadata);
+ case MAP:
+ case OTHER:
+ throw new UnsupportedOperationException(String.format("Unsupported data type: %s", relDataType.getSqlTypeName()));
+ default:
+ if (componentType.isStruct()) {
+ // for the case when nested type is struct, it should be placed into repeated map
+ return MetadataUtils.newMapArray(name, childColumnMetadata.tupleSchema());
+ } else {
+ // otherwise creates column metadata with repeated data mode
+ return new PrimitiveColumnMetadata(
+ MaterializedField.create(name,
+ Types.overrideMode(
+ TypeInferenceUtils.getDrillMajorTypeFromCalciteType(componentType),
+ TypeProtos.DataMode.REPEATED)));
+ }
+ }
+ }
+
+ /**
+ * Returns {@link MapColumnMetadata} column metadata created based on specified {@code RelDataType relDataType} with
+ * converted to {@link ColumnMetadata} {@code relDataType}'s children.
+ *
+ * @param name name of the filed
+ * @param relDataType {@link RelDataType} the source of the children for resulting schema
+ * @return {@link MapColumnMetadata} column metadata
+ */
+ private static MapColumnMetadata getStructMetadata(String name, RelDataType relDataType) {
+ TupleMetadata mapSchema = new TupleSchema();
+ relDataType.getFieldList().stream()
+ .map(field -> getColumnMetadata(field.getName(), field.getType()))
Review Comment:
What would happen here if you have a nested map? IE something like this:
```
{
"field1": {
"nested_field1":"something",
"nested_field2": {
"really_nested":"something",
"real_nested2":"something_else"
}
}
}
```
Or are nested schemata not supported for `VALUES()` operator?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi merged pull request #2539: DRILL-8216: Use EVF-based JSON reader for Values operator
Posted by GitBox <gi...@apache.org>.
vvysotskyi merged PR #2539:
URL: https://github.com/apache/drill/pull/2539
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org