Support APPEND for insert_existing_partitions_behavior and align default with immutable_partitions #17597
wongquijote
started this conversation in
General
Replies: 2 comments 1 reply
0 replies
|
@mbasmanova @xiaoxmeng : Wanted to bring your attention to this issue. Seems like Velox honors HiveConfig for immutablePartitions for un-partitioned tables while it ignores it for partitioned tables. Its odd that we consider immutablePartitions for unpartitioned tables (which shouldn't have partitions) vs partitioned tables that do. velox/velox/connectors/hive/HiveDataSink.cpp Line 837 in 7e73421 We want to add Append behavior for partitioned tables and use immutablePartitions config for parity with Presto Java if we can. Is there any reason for ignoring immutablePartitions config when inserting into partitioned tables ? I would imagine this config is not set in your clusters then. |
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
In Java Presto, HiveClientConfig.getInsertExistingPartitionsBehavior() couples the default to immutablePartitions: it returns APPEND when partitions are mutable, and ERROR/OVERWRITE when they are immutable.
Velox C++ diverges in two related ways:
- InsertExistingPartitionsBehavior enum has only kError and kOverwrite (HiveConfig.cpp:30,33).
- stringToInsertExistingPartitionsBehavior only parses "ERROR" and "OVERWRITE".
- The switch in HiveDataSink::getUpdateMode() (HiveDataSink.cpp:837) has no kAppend case — the default: arm throws VELOX_UNSUPPORTED.
Interestingly, the unpartitioned existing-table path already implements the Java-style behavior: returns kAppend when mutable, fails when immutable (HiveDataSink.cpp:854-857). So immutablePartitions is consulted there — just not for the partitioned-table path, where it's never read.
I'd like to support APPEND for existing partitions. Before I make changes, I wanted to surface this asymmetry and check whether it was intentional. My proposed direction:
All reactions