github-actions[bot] commented on code in PR #66413:
URL: https://github.com/apache/doris/pull/66413#discussion_r3749283719
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1574,15 +1586,52 @@ private long computeSplitWeight(DataSplit dataSplit) {
* must still be allowed native.
*
* <p>{@code forceJniScanner} is the user/session escape hatch ({@code SET
force_jni_scanner=true},
- * read via {@link #isForceJniScannerEnabled}): when set, every
native-eligible split is routed to
- * JNI to dodge native-reader bugs. Default false, so normal reads are
unaffected.
+ * read via {@link #isForceJniScannerEnabled}): when set, every
native-eligible non-Variant split is
+ * routed to JNI to dodge native-reader bugs. Variant projections stay
native because JNI cannot
+ * carry Variant columns. Default false, so normal reads are unaffected.
*
* <p>Extracted as a pure static so the correctness-critical routing
decision is unit-testable
* with real {@link RawFile}s, without driving a full Paimon {@code
ReadBuilder}/{@code TableScan}.
*/
static boolean shouldUseNativeReader(boolean forceJni, boolean
forceJniScanner,
Optional<List<RawFile>> optRawFiles) {
- return !forceJni && !forceJniScanner &&
supportNativeReader(optRawFiles);
+ return shouldUseNativeReader(forceJni, forceJniScanner, false,
optRawFiles);
+ }
+
+ static boolean shouldUseNativeReader(boolean forceJni, boolean
forceJniScanner,
+ boolean hasVariantProjection, Optional<List<RawFile>> optRawFiles)
{
+ // JNI has no Variant carrier, so Variant projections override the
debugging force-JNI knobs
+ // while still requiring an ORC/Parquet split that the native reader
can consume.
+ return supportNativeReader(optRawFiles)
Review Comment:
[P1] Preserve the semantic force-JNI fence for Variant scans
With `hasVariantProjection`, this returns true even when `forceJni` is set.
That flag is not a debug preference: it marks `$binlog`, `$audit_log`, and
`$row_tracking`, whose pack/merge, row-kind, and sequence behavior cannot be
reproduced by raw ORC/Parquet reads (the existing non-Variant routing test
documents this). Projecting a Variant from one of those relations now silently
routes it to the native reader and changes results. The same branch also
accepts `.orc`, while BE only installs Paimon Variant schema overrides for
`FileFormat::PARQUET`. Keep handle-level `forceJni` unconditional and reject
unsupported Variant system-table/ORC scans (or add the missing carriers); only
the user session force flag can be considered separately.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriteSchemaContext.java:
##########
@@ -202,13 +202,12 @@ private static SortOrder bindSortOrder(SortOrder
sortOrder, Schema schema, Strin
private static void validateWriterMetadataSources(
Schema schema, PartitionSpec partitionSpec, SortOrder sortOrder,
String tableName) {
- Map<Integer, Types.NestedField> topLevelFields =
schema.columns().stream()
-
.collect(ImmutableMap.toImmutableMap(Types.NestedField::fieldId, field ->
field));
for (PartitionField field : partitionSpec.fields()) {
- if (!topLevelFields.containsKey(field.sourceId())) {
+ // Iceberg permits a nested primitive field as a partition source;
field IDs are schema-wide.
+ if (schema.findField(field.sourceId()) == null) {
Review Comment:
[P1] Fence nested partition writes from old backends
This change makes nested Iceberg partition source IDs newly admissible, but
there is no execution-version gate before the plan can run on an old BE. The
removed writer implementation builds its source map from top-level schema IDs
only, so it throws `outside writer schema` for this nested ID; MERGE also
relies on the new optional `source_field_path` field that an old BE ignores.
During a rolling upgrade, INSERT, UPDATE, or data-writing MERGE on a
nested-partition spec can therefore fail depending on backend placement. Fence
this capability at the query-wide BE execution version (or provide an
old-BE-compatible source expression) and add a version-11 compatibility test.
--
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]
