[GLUTEN-10745][VL] Refactor parseScanSplitInfo for VeloxPlanConverter#10742
[GLUTEN-10745][VL] Refactor parseScanSplitInfo for VeloxPlanConverter#10742beliefer wants to merge 1 commit intoapache:mainfrom
Conversation
beliefer
changed the title
| partitionColumnMap.emplace(partitionColumn.key(), partitionColumn.value()); | ||
| } | ||
| splitInfo->partitionColumns.emplace_back(partitionColumnMap); | ||
| splitInfo->partitionColumns.emplace_back(std::move(partitionColumnMap)); |
There was a problem hiding this comment.
Please check if the partitionColumnMap isEmpty first
There was a problem hiding this comment.
std::move is safe here even if partitionColumnMap is empty.
emplace_back could accept the empty partitionColumnMap.
The overhead of std::move is very small and check if the partitionColumnMap isEmpty brings extra overhead.
I will change the code as you said if you insist. :)
There was a problem hiding this comment.
For the interface, if the table is not a partitioned table, we should not set the field, I miss an issue before, #10622
There was a problem hiding this comment.
What happened if we set the field with empty partitionColumnMap?
| metadataColumnMap.emplace(metadataColumn.key(), metadataColumn.value()); | ||
| } | ||
| splitInfo->metadataColumns.emplace_back(metadataColumnMap); | ||
| splitInfo->metadataColumns.emplace_back(std::move(metadataColumnMap)); |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
GA, please hold this PR. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
2 participants
What changes are proposed in this pull request?
This PR proposes to improve
parseScanSplitInfo.Fixes: #10745
How was this patch tested?
GA tests.