ramitg254 commented on code in PR #6337:
URL: https://github.com/apache/hive/pull/6337#discussion_r2916139840
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java:
##########
@@ -609,25 +609,30 @@ public boolean equals(Object obj) {
return true;
}
- public List<FieldSchema> getPartCols() {
+ public List<FieldSchema> getPartCols(boolean needNonNativeParts) {
Review Comment:
hi, please check the updated implementation where I have used two separate
methods and removed this boolean flag. These are the reasons why I didn't
updated getPartCols implementation in the first place for non native tables:
1. there are many usages of getPartCols() where it should return 0 partition
keys which is required for those implementations like in
MergeSemanticAnalyzer.java, couple of places with stats gathering and alter
commands , also while conversion to iceberg table for new table
hasNonNativePartitionSupport is true but it is missing table type property
which should be iceberg as a sym‎bol of registered iceberg table and will fall
into exception if updated implementation of getPartCols() is directly used.
2. as mentioned in 1. to resolve this if we are going with 1 implentation of
getPartCols() it will lead to introduction of several ifs in too many to handle
those scenarios and that would only if we have complete confidence that tests
have complete coverage for all those implementations.
so I think 2 separate method approach is appropriate here in which
getPartCols is just renamed to getNativePartCols having same earlier
implementation and getSupportedPartCols which have non Native support as well
and can be used in places which do not affect any other logical implementation.
--
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]
