Skip to content

Branch for testinf Phoenix-6984, phoenix-6983 and phoenix 6959 together on 5.1#1631

Closed
stoty wants to merge 3 commits intoapache:5.1from
stoty:PHOENIX-6984-5.1-PHOENIX-6983-5.1-PHOENIX-6959
Closed

Branch for testinf Phoenix-6984, phoenix-6983 and phoenix 6959 together on 5.1#1631
stoty wants to merge 3 commits intoapache:5.1from
stoty:PHOENIX-6984-5.1-PHOENIX-6983-5.1-PHOENIX-6959

Conversation

@stoty
Copy link
Copy Markdown
Contributor

@stoty stoty commented Jun 20, 2023

No description provided.

@stoty
Copy link
Copy Markdown
Contributor Author

stoty commented Jun 20, 2023

This contains backports of Phoenix-6984 and phoenix-6983, and adds PHOENIX-6959 on top of it.
Phoenix-6959 is specific to 5.1, as it backports some fixes from master.

We need to review these together, or do the reviews and commits on two or three passes.

* the index table to the data table.
*/
// Reset the state changes from the attempt above
indexTableRef.setHinted(false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

without this reset, the compiler might still end up using uncovered index right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We'd use the skip-scan-join plan with the projector for the server merge plan, which doesn't work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For example we saw

Error: ERROR 504 (42703): Undefined column. columnName=BILLING_ORDER.0:PERIOD_END (state=42703,code=504)
org.apache.phoenix.schema.ColumnNotFoundException: ERROR 504 (42703): Undefined column. columnName=BILLING_ORDER.0:PERIOD_END

which would refer to the projected data table in the server merge plan, but the actual Table is unprojected data table in the skip-join-scan, which doesn't have a column of that name.

Resetting those flags makes sure that we generate and use the correct projector.

Comment on lines +158 to +161
String expected =
"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + indexTableName + " ['a']\n" +
" SERVER MERGE [0.K3]\n" +
" SERVER FILTER BY FIRST KEY ONLY";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we not need NO_INDEX_SERVER_MERGE with INDEX("dt" "it") for this to be changed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With PHOENIX-6959 the behaviour is the same as 5.2, and these changes are taken from 5.2 branch.

I have added a single new test case for the NO_INDEX_SERVER_MERGE option below.

Comment on lines +513 to +515
if (!(tableRef.getTable().getIndexType() == IndexType.LOCAL || isHintedGlobalIndex(tableRef))) {
throw e;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for else case, would a DEBUG log be helpful?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is again synced from 5.2, which does not have a debug statement there.
This applies to all server merged columns, so it would generate a log line for each merged column, which sounds excessive.

@stoty stoty closed this Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants