morningman commented on code in PR #70369:
URL: https://github.com/apache/doris/pull/70369#discussion_r4574537427
##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/pushdown/ConnectorLiteral.java:
##########
@@ -29,9 +29,10 @@
/**
* A literal value expression.
*
- * <p>The value is a standard Java type. The engine produces exactly these
eight shapes — see the package
+ * <p>The value is a standard Java type. The engine produces exactly these
nine shapes — see the package
* javadoc (Rule 4) for the full Doris-type-to-Java-class table: {@code null},
{@code Boolean}, {@code Long},
- * {@code Double}, {@code BigDecimal}, {@code String}, {@code LocalDate},
{@code LocalDateTime}.</p>
+ * {@code Double}, {@code BigDecimal}, {@code String}, {@code LocalDate},
{@code LocalDateTime}, and for an
+ * {@code ARRAY} an unmodifiable {@code List} whose elements take these same
shapes.</p>
Review Comment:
Thanks. I'm keeping 30.0: the major tracks the SPI surface, and this PR does
not change it.
- The pom comment and `ApiVersionGate` define a major as a change to the set
of SPI elements a plugin can see, additions included, and
`ConnectorPluginSurfaceTest#connectorApiMajorTracksTheRecordedSurfaceChange`
ties it to the recorded baseline; master bumps it exactly when that baseline
changes. This PR adds, removes and changes no type, method or enum constant.
What changes is the documented value of `ConnectorLiteral#getValue()` for
ARRAY, which used to be display text.
- The pair you describe needs a JDBC plugin built before this PR next to an
FE built after it, and no release can ship one: the connector SPI exists only
on master (branch-4.x has no fe-connector), and build.sh builds every in-tree
connector with the FE and deploys it under `fe/plugins/connector/`.
- That text was not a working encoding to preserve: PostgreSQL and
ClickHouse reject `'[a, b]'`, and a remote Doris reads elements containing `, `
or NULL back as other elements; `test_jdbc_array_literal_predicate` fails on
master for each.
#70338 and #70372 take 31.0 for real surface changes.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ExprToConnectorExpressionConverter.java:
##########
@@ -301,12 +302,26 @@ private static ConnectorExpression
convertLiteral(LiteralExpr literal) {
return new ConnectorLiteral(ct, ((StringLiteral)
literal).getValue());
} else if (literal instanceof DateLiteral) {
return convertDateLiteral((DateLiteral) literal, ct);
+ } else if (literal instanceof ArrayLiteral) {
+ return new ConnectorLiteral(ct, arrayElements((ArrayLiteral)
literal));
Review Comment:
Thanks, confirmed, and fixed in 3162b0e5eab.
`PluginDrivenScanNode.convertPredicate` hands every conjunct to `applyFilter`
(the cast switch does not apply there), the converter strips the CAST, and
Hudi's `extractLiteralValue` turned the element list into `[true]`. Master's
text `[1]` matched the BE cast only by accident: for `CAST(['a'] AS STRING)`
both forms differ from BE's `["a"]`, so master already pruned that partition
away.
A literal of type ARRAY, MAP or STRUCT is never a partition value, so Hudi
and its Hive twin now skip it: an EQ with one is not a partition predicate, and
an IN prunes only when every item is a scalar literal, since dropping just that
item would narrow the list. The conjunct stays with Doris like any predicate
the pruner does not understand.
`HudiPartitionPruningTest#complexLiteralIsNeverAPartitionValue` and
`HiveConnectorMetadataPartitionPruningTest#testComplexLiteralIsNeverAPartitionValue`
fail without the check: the `[1]` partition is dropped.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
