Skip to content

HIVE-29478: FileSinkOperator shouldn't check for isMMTable for everyrow being processed#6341

Open
nareshpr wants to merge 1 commit intoapache:masterfrom
nareshpr:HIVE-29478
Open

HIVE-29478: FileSinkOperator shouldn't check for isMMTable for everyrow being processed#6341
nareshpr wants to merge 1 commit intoapache:masterfrom
nareshpr:HIVE-29478

Conversation

@nareshpr
Copy link
Contributor

What changes were proposed in this pull request?

conf.isMMTable() shouldn't be invoked for every row being processed. This came up as hotspot in ETL file write flow.

Why are the changes needed?

It improves write performance, specifically properties to map conversion is not required to validate a single property for every row being processed.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tests were not added, as its performance improvement.

@sonarqubecloud
Copy link

Copy link
Contributor

@abstractdog abstractdog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending tests

@abstractdog
Copy link
Contributor

@nareshpr : approved, but I've just realized there are a few more occurrences of Maps.fromProperties, can you take care of those please?

@nareshpr
Copy link
Contributor Author

yes, i will remove all Map conversion in AcidUtils.

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.

3 participants