github-actions[bot] commented on code in PR #65085:
URL: https://github.com/apache/doris/pull/65085#discussion_r3526249360
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -2945,10 +2945,10 @@ public static boolean isEagerAggregationOnJoin() {
public boolean hiveParquetUseColumnNames = true;
@VarAttrDef.VarAttr(name = HIVE_ORC_USE_COLUMN_NAMES,
affectQueryResultInExecution = true,
- description = {"默认情况下按名称访问 Orc 列。将此属性设置为“false”可按 Hive
表定义中的序号位置访问列。",
- "Access Parquet columns by name by default. Set this
property to `false` to access columns "
- + "by their ordinal position in the Hive table
definition."})
- public boolean hiveOrcUseColumnNames = true;
+ description = {"默认情况下按照 Hive 表定义中的序号位置访问列。将此属性设置为“true”可按名称访问 Orc
列 。",
+ "By default, columns are accessed based on their ordinal
position in the Hive table definition."
+ + " Set this property to `true` to access ORC
columns by name."})
+ public boolean hiveOrcUseColumnNames = false;
Review Comment:
The new default makes ordinary Hive ORC scans take the ordinal path, but
that path cannot currently compute ORC column ids.
`HiveOrcReader::_create_column_ids_by_top_level_col_index()` looks up each slot
by `slot->col_pos()`, while FE serializes every `TSlotDescriptor` with
`columnPos = -1` and BE copies that into `SlotDescriptor::_col_pos`. With the
default now `false`, `column_ids` stays empty for normal external scan slots,
so `OrcReader` falls back from `includeTypes()` to the `_read_file_cols` name
list and loses the column-id/nested pruning path that name mode still has.
Please either derive these ids from the same `column_idxs` mapping used for
ordinal table-info construction, or serialize real table positions before
changing the default. The thrift contract should also be kept in sync:
`PaloInternalService.thrift` still documents/defaults
`hive_orc_use_column_names` as `true`.
##########
regression-test/suites/external_table_p0/hive/test_different_column_orders.groovy:
##########
@@ -28,6 +28,7 @@ suite("test_different_column_orders", "p0,external") {
"""
}
def q_orc = {
+ sql """ set hive_orc_use_column_names = true """
qt_q01 """
Review Comment:
This pins the suite back to the old name-based behavior, so it does not
protect the default change this PR is making. With `hive_orc_use_column_names`
now defaulting to `false`, the regression should also run this
different-column-order ORC table with the default value, or explicitly with
`set hive_orc_use_column_names = false`, and record the expected positional
result. Keeping only this `true` block would still pass if the default in
`SessionVariable` accidentally stayed `true`.
--
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]