HIVE-29782:ALTER VIEW ADD PARTITION does not honor non string partition columns - #6658
Open
ramitg254 wants to merge 2 commits into
Open
HIVE-29782:ALTER VIEW ADD PARTITION does not honor non string partition columns#6658ramitg254 wants to merge 2 commits into
ramitg254 wants to merge 2 commits into
Conversation
ramitg254
changed the title
asf-ci-hive
added
tests failed
tests pending
tests passed
and removed
tests pending
tests failed
labels
uros-b
approved these changes
Aug 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes HIVE-29782 by ensuring ALTER VIEW ... ADD PARTITION generates correctly-typed partition literals (instead of forcing all partition values to be treated as strings), so partitions can be added for views partitioned on non-string primitive columns.
Changes:
- Update
AlterViewAddPartitionAnalyzerto format partition literals based on the partition column type. - Add validation in
CreateViewAnalyzerto reject non-primitive partition columns for partitioned views. - Add new positive/negative qtests covering primitive (int/boolean/date/decimal) and non-primitive (struct) partition columns.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AlterViewAddPartitionAnalyzer.java | Generate type-aware literals for the internal validation query used by ALTER VIEW ADD PARTITION. |
| ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java | Validate that view partition columns are primitive types. |
| ql/src/test/queries/clientpositive/partitioned_view_add_parts.q | New positive coverage for adding partitions to views with non-string primitive partition columns. |
| ql/src/test/results/clientpositive/llap/partitioned_view_add_parts.q.out | Expected output for the new positive test. |
| ql/src/test/queries/clientnegative/partitioned_view_add_parts_complex.q | New negative coverage: reject non-primitive partition column in CREATE VIEW ... PARTITIONED ON. |
| ql/src/test/results/clientnegative/partitioned_view_add_parts_complex.q.out | Expected error output for the new negative test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+109
to
+115
| private static String formatPartitionLiteral(FieldSchema partCol, String partSpecValue) { | ||
| TypeInfo typeInfo = TypeInfoFactory.getPrimitiveTypeInfo(partCol.getType()); | ||
| ObjectInspector partColOI = TypeInfoUtils.getStandardJavaObjectInspectorFromTypeInfo(typeInfo); | ||
| Object converted = ObjectInspectorConverters.getConverter( | ||
| PrimitiveObjectInspectorFactory.javaStringObjectInspector, partColOI).convert(partSpecValue); | ||
| return new ExprNodeConstantDesc(typeInfo, converted).getExprString(); | ||
| } |
…on columns Change-Id: I56fa674c2d7c8db6187d75e9fd2f5d39a3ec766e Change-Id: I790b126f63e73d03dc0ef2d04ed1e71d37f79028
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
labels
Change-Id: Ie22bcfb10d777360d9ee013d8f2ccba65c91f218
asf-ci-hive
added
tests unstable
tests pending
and removed
tests unstable
tests pending
labels
|
ayushtkn
reviewed
Aug 13, 2026
ayushtkn
left a comment
ayushtkn
left a comment
Member
There was a problem hiding this comment.
Thanx @ramitg254 for working on this. Have dropped a couple of comments
Comment on lines
+193
to
+198
| try { | ||
| TypeInfoFactory.getPrimitiveTypeInfo(fieldSchema.getType()); | ||
| } catch (Exception e) { | ||
| throw new SemanticException(ErrorMsg.PARTITION_COLUMN_NON_PRIMITIVE.getMsg() + " Found " | ||
| + columnName + " of type: " + fieldSchema.getType()); | ||
| } |
Member
There was a problem hiding this comment.
can we instead of catching Exception and throwing do
TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString(fieldSchema.getType());
if (typeInfo.getCategory() != ObjectInspector.Category.PRIMITIVE) {
throw new SemanticException(ErrorMsg.PARTITION_COLUMN_NON_PRIMITIVE.getMsg() + " Found "
+ columnName + " of type: " + fieldSchema.getType());
}
| } else { | ||
| where.append(" AND "); | ||
| } | ||
| FieldSchema partCol = table.getColumnByName(entry.getKey()); |
Member
There was a problem hiding this comment.
Shoudln't this be?
FieldSchema partCol = table.getPartColByName(entry.getKey());
can u check how it is working currently, the logic inside getPartColByName handles nonNativePartition as well, see if it is working
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change-Id: I56fa674c2d7c8db6187d75e9fd2f5d39a3ec766e
What changes were proposed in this pull request?
Fixed the behaviour of ALTER VIEW ADD PARTITION for non string partition columns
Why are the changes needed?
in
postProcessinAlterViewAddPartitionAnalyzer.javaall partition values are treated as string literals which prevent it from adding partition in parition columns are other than string columns.Steps to reproduce are explained on jira: https://issues.apache.org/jira/browse/HIVE-29782
Does this PR introduce any user-facing change?
No
How was this patch tested?
Added ut and results of ci