Skip to content

HIVE-29413: Generalise column related APIs in Table.java#6413

Open
ramitg254 wants to merge 45 commits into
apache:masterfrom
ramitg254:HIVE-29413
Open

HIVE-29413: Generalise column related APIs in Table.java#6413
ramitg254 wants to merge 45 commits into
apache:masterfrom
ramitg254:HIVE-29413

Conversation

@ramitg254

@ramitg254 ramitg254 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

added getEffectivePartCols() in most places possible to avoid code duplication.

Why are the changes needed?

getPartCols() does not have support for iceberg tables.

Does this PR introduce any user-facing change?

No

How was this patch tested?

ci tests and local build

@deniskuzZ

Copy link
Copy Markdown
Member

@ramitg254 please take a look: 9e7535c. I would suggest following similar approach

@ramitg254

ramitg254 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

9e7535c

but here we are creating separate method getEffectivePartCols() and leaving getPartCols() as it is, which as per our discussion on that closed pr we shouldn't do that, and only go ahead with updating getPartCols()

@deniskuzZ

deniskuzZ commented Apr 10, 2026

Copy link
Copy Markdown
Member

9e7535c

but here we are creating separate method getEffectivePartCols() and leaving getPartCols() as it is, which as per our discussion on that closed pr we shouldn't do that, and only go ahead with updating getPartCols()

Where did I say that? The ask was to keep the original method unchanged. same here

@ramitg254

ramitg254 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

oh I got confused due to this comment: #6337 (comment) in which getSupportedPartCols() was just separate method similar to getEffectivePartCols()

@ramitg254

ramitg254 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

I am fine with that earlier approach as well but recently I saw this one: https://issues.apache.org/jira/browse/HIVE-29525 so I thought we should have unified getPartCols() and getCols() which gives similar results as native hive tables as first step towards solving this after that those plan logics can be taken care of later on when that ticket will be addressed.
So I was first focussing on making getPartCols() unified for iceberg tables as well.

please share your thoughts on this idea

ramitg254 added 29 commits July 1, 2026 00:37
# Conflicts:
#	iceberg/iceberg-handler/src/test/results/positive/llap/iceberg_bucket_map_join_7.q.out
Change-Id: I09ffba356ac47e3416c8b6717e8671d2cb6432b8
Change-Id: I6812d068be702edf01b158c3e7eacadeadbd5c2b
Change-Id: I6f5d81c5be00e080753efd5e8157b14fcfde2dde
Change-Id: I5a95791b4d756fe949ee4697204be9de3025c6b5
Change-Id: I852cf156a1a5ac20525bb9376f4cbb5783b8e1e3
Change-Id: Ib1542e38c65c7811074b97d7da7aaf780f3895dd
Change-Id: Ie3440239248870fb44c6e956d5be10271b28a1a0
Change-Id: Ic8f6d6c391ba49e12de7d04b7bb20542d879b0a3
Change-Id: I2017f7c771f8f611b5d8900d3e0276817842b41f
Change-Id: I87dc65eb942e3db1b1a0d9f2608f1b88b44b11e6
Change-Id: Ic0fefede8b079205c5580a31f59f195b6b1e94e0
Change-Id: I5a34151d5ec0d4e1759eee5fcc6339b5fd6b92d3
Change-Id: I0b130777712dd05df53a1ca6ed80ea4c5abd0187
Change-Id: Id1ce8250addc158a36887d4c8538ee2ab18b6f39
Change-Id: Ic5ec07bc8c9c1c656407fcaec08904792dd61093
Change-Id: I518f8baac535b65fad0295bd1c892457c904d8e9
Change-Id: I4d327a819ab27f222ca2d085cbefd71811ab17f4
Change-Id: Ib5a6777a2fce8fecb7ff172dc5b6088c31f25918
Change-Id: I670d02602f2e9cec2b312f57dcd70265be05c67e
@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants