github-actions[bot] commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r4014740664
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -1636,6 +1641,8 @@ Status
TableReader::create_file_reader(std::unique_ptr<FileReader>* reader) {
const bool enable_mapping_varbinary = _scan_params != nullptr &&
_scan_params->__isset.enable_mapping_varbinary &&
_scan_params->enable_mapping_varbinary;
+ const std::optional<std::string> hive_parquet_time_zone =
Review Comment:
[P1] Isolate condition-cache entries by the INT96 timezone contract
This request-local value changes how INT96 rows evaluate predicates, but it
is passed only to `ParquetReader`; the process-wide external condition-cache
key still consists of file identity, byte range, the session-variable seed, and
the expression digest. Scanning the same file and predicate through two HMS
catalogs or TVFs with wall-clock versus (for example) `Asia/Shanghai` semantics
can therefore reuse an all-false granule bitmap and skip the second scan's
matching rows before decoding. Since `enable_condition_cache` defaults on,
please include the normalized absent/empty/named INT96 contract in the cache
identity, or disable condition-cache reuse for affected timestamp predicates.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java:
##########
@@ -103,18 +104,20 @@ public Map<String, String> getProperties() {
}
/**
- * @return true if varbinary mapping is enabled, false otherwise
+ * @return true because external binary types always retain their binary
semantics
*/
+ @Deprecated
public boolean getEnableMappingVarbinary() {
- return Boolean.parseBoolean(getOrDefault(ENABLE_MAPPING_VARBINARY,
"false"));
+ return true;
Review Comment:
[P1] Preserve existing SQL support before forcing VARBINARY
This makes binary mapping unconditional for every existing external catalog,
but the unchanged analyzers still reject `VARBINARY` in comparison predicates,
`GROUP BY`, equi-joins, views, CTAS, and MTMV definitions. Queries and derived
objects that worked while Hive/Iceberg/Paimon/JDBC binary columns mapped to
`STRING` therefore fail after upgrade, and a persisted false setting can no
longer opt out. The changed tests either drop/hex-encode those columns or test
JDBC pushdown only after an expression already exists, so they do not protect
this compatibility. Please retain the compatibility mapping until these
consumers support `VARBINARY`, or add that support with end-to-end regressions
over the original binary columns.
--
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]
