github-actions[bot] commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r3998836236
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/FileQueryScanNode.java:
##########
@@ -247,9 +249,15 @@ protected void initSchemaParams() throws UserException {
setColumnPositionMapping();
// For query, set src tuple id to -1.
params.setSrcTupleId(-1);
- // Set enable_mapping_varbinary from catalog or TVF
params.setEnableMappingVarbinary(getEnableMappingVarbinary());
params.setEnableMappingTimestampTz(getEnableMappingTimestampTz());
+ // The marker makes an omitted timezone an explicit wall-clock choice
while old FE plans
+ // remain distinguishable during a BE-first rolling upgrade.
+
params.setParquetTimestampSemanticsVersion(FileFormatUtils.PARQUET_TIMESTAMP_SEMANTICS_VERSION);
Review Comment:
[P1] Keep phase-two TopN fetch on the versioned timestamp reader
This marker forces the initial Parquet scan onto V2, but TopN lazy
materialization later bypasses that decision:
`RowIdStorageReader::read_external_row_from_file_mapping` directly creates
legacy `FileScanner` for its phase-two row fetch. If a timestamp is deferred
(for example, `SELECT id, ts ... ORDER BY id LIMIT 10`), V1 ignores this marker
and `hive_parquet_time_zone` and instead decodes with the session timezone.
Phase one and phase two can therefore apply different semantics to the same
file. Please route the row-id fetch through a V2-capable reader or disable this
optimization for versioned timestamp scans, and cover it with a deferred
timestamp regression.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcPostgreSQLClient.java:
##########
@@ -188,8 +188,7 @@ protected Type jdbcTypeToDoris(JdbcFieldSchema fieldSchema)
{
case "jsonb":
return ScalarType.createStringType();
case "bytea": //
https://www.postgresql.org/docs/12/datatype-binary.html#DATATYPE-BINARY-TABLE
- return enableMappingVarbinary ?
ScalarType.createVarbinaryType(fieldSchema.requiredColumnSize())
- : ScalarType.createStringType();
+ return
ScalarType.createVarbinaryType(fieldSchema.requiredColumnSize());
Review Comment:
[P1] Render VARBINARY IN literals in the target JDBC dialect
With `bytea` now always mapped to VARBINARY, a same-typed predicate such as
`bytea_col IN (X'DEADBEEF')` passes Nereids coercion and reaches JDBC pushdown.
`JdbcScanNode.conjunctExprToString` has no binary-specific branch, so
`toExternalSql` emits Doris's generic `X'<hex>'`; PostgreSQL interprets that as
a bit string rather than `bytea`, and the remote predicate fails instead of
returning rows. The analogous forced SQL Server and Oracle mappings also need
their own binary syntax. Please render binary literals per dialect or decline
pushdown for these conjuncts so Doris can evaluate them locally, and add
predicate regressions for the newly forced mappings.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -2514,12 +2520,11 @@ private boolean
isPositionDeletesPartitionColumnRequested() {
private String getPartitionDataObjectJson(PartitionData partitionData,
PartitionSpec partitionSpec,
List<NestedField> outputPartitionFields) throws UserException {
List<NestedField> partitionTypes =
partitionData.getPartitionType().asNestedType().fields();
- boolean enableMappingVarbinary = getEnableMappingVarbinary();
for (int i = 0; i < partitionTypes.size(); i++) {
Type type = partitionTypes.get(i).type();
if (partitionData.get(i) != null && (type.typeId() ==
Type.TypeID.BINARY
|| type.typeId() == Type.TypeID.FIXED
- || (type.typeId() == Type.TypeID.UUID &&
enableMappingVarbinary))) {
+ || type.typeId() == Type.TypeID.UUID)) {
Review Comment:
[P2] Preserve UUID partitions in the position_deletes metadata table
Because this PR makes Iceberg UUID columns permanently VARBINARY, this
unconditional branch turns the former opt-in limitation into a failure for
every non-null UUID identity partition whenever `$position_deletes.partition`
is projected. The previous default path could serialize UUID as text; there is
now no supported representation at all. Please add a binary-safe 16-byte
partition transport (and a UUID-partition regression) instead of making this
metadata query unconditionally fail.
--
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]
