Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix backward scan flag for parallel queries #1604

Conversation

rl626
Copy link
Contributor

@rl626 rl626 commented Jul 11, 2023

Description

Currently there is a bug in Babelfish that allows the backward scan flag to be set for parallel queries. However, backward scans cannot be parallelized. This bug results in assertion failures in the Postgres engine and leads Babelfish queries to crash.

Issues Resolved

In this commit, we fix the issue by disallowing backward scan flag to be set when a query is in parallel mode. This issue occurs during the implementation of full text search in Babelfish. Fixing the issue unblocks full text search implementation.

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Robin Li added 5 commits July 11, 2023 15:24
Currently there is a bug in Babelfish that allows the backward scan flag
to be set for parallel queries. However, backward scans cannot be
parallelized. This bug results in assertion failures in the Postgres engine and
leads Babelfish queries to crash. In this commit, we fix the issue by
disallowing backward scan flag to be set when a query is in parallel
mode. This issue occurs during the implementation of full text search in
Babelfish. Fixing the issue unblocks full text search implementation.

Signed-off-by: Robin Li <[email protected]>
select set_config('min_parallel_table_scan_size', 0, false);
GO

select a, count(*) from t_babel4261 group by a order by 2; -- should not crash
Copy link
Contributor

Choose a reason for hiding this comment

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

could you use

SET BABELFISH_STATISTICS PROFILE ON

at this test place, to explicit show it's a parallel query plan at here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

select a, count(*) from t_babel4261 group by a order by 2; -- should not crash
GO

DROP TABLE t_babel4261;
Copy link
Contributor

Choose a reason for hiding this comment

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

select set_config('parallel_setup_cost', 0, false);
select set_config('parallel_tuple_cost', 0, false);
select set_config('min_parallel_table_scan_size', 0, false);

could you set those three parameters back to default value in this place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure how to set them back since I don't know what the default value is. Also, it seems that in this test

select set_config('parallel_setup_cost', '0', false)
they are not set back. So I guess not setting them back is okay?

Copy link
Contributor

Choose a reason for hiding this comment

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

No it's not, 3294 didn't do the right thing in here

EXEC sp_babelfish_configure 'babelfishpg_tsql.xxxx', 'default';

Would you try this to reset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried doing that, but it doesn't seem to work:

1> EXEC sp_babelfish_configure 'babelfishpg_tsql.parallel_setup_cost', 'default';
2> go
Msg 33557097, Level 16, State 1, Server BABELFISH, Line 1
unknown configuration: babelfishpg_tsql.parallel_setup_cost

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have done this by putting set_config in a transaction and make the third parameter true, so that the configurations will be set back to default when the transaction is committed.

@forestkeeper forestkeeper merged commit e0ab145 into babelfish-for-postgresql:BABEL_3_X_DEV Jul 12, 2023
rl626 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jul 12, 2023
…#1604)

* Fix backward scan flag for parallel queries

Currently there is a bug in Babelfish that allows the backward scan flag
to be set for parallel queries. However, backward scans cannot be
parallelized. This bug results in assertion failures in the Postgres engine and
leads Babelfish queries to crash. In this commit, we fix the issue by
disallowing backward scan flag to be set when a query is in parallel
mode. This issue occurs during the implementation of full text search in
Babelfish. Fixing the issue unblocks full text search implementation.

Task: BABEL-4261, BABEL-4281
Signed-off-by: Robin Li <[email protected]>
rohit01010 pushed a commit to rohit01010/babelfish_extensions that referenced this pull request Nov 9, 2023
…#1604)

* Fix backward scan flag for parallel queries

Currently there is a bug in Babelfish that allows the backward scan flag
to be set for parallel queries. However, backward scans cannot be
parallelized. This bug results in assertion failures in the Postgres engine and
leads Babelfish queries to crash. In this commit, we fix the issue by
disallowing backward scan flag to be set when a query is in parallel
mode. This issue occurs during the implementation of full text search in
Babelfish. Fixing the issue unblocks full text search implementation.

Task: BABEL-4261, BABEL-4281
Signed-off-by: Robin Li <[email protected]>
rohit01010 pushed a commit to rohit01010/babelfish_extensions that referenced this pull request Nov 9, 2023
rohit01010 pushed a commit to rohit01010/babelfish_extensions that referenced this pull request Nov 9, 2023
…#1604)

Currently there is a bug in Babelfish that allows the backward scan flag
to be set for parallel queries. However, backward scans cannot be
parallelized. This bug results in assertion failures in the Postgres engine and
leads Babelfish queries to crash. In this commit, we fix the issue by
disallowing backward scan flag to be set when a query is in parallel
mode. This issue occurs during the implementation of full text search in
Babelfish. Fixing the issue unblocks full text search implementation.

Task: BABEL-4261, BABEL-4281
Signed-off-by: Robin Li <[email protected]>
Deepesh125 pushed a commit that referenced this pull request Nov 10, 2023
* Fix backward scan flag for parallel queries (#1604)

Currently there is a bug in Babelfish that allows the backward scan flag
to be set for parallel queries. However, backward scans cannot be
parallelized. This bug results in assertion failures in the Postgres engine and
leads Babelfish queries to crash. In this commit, we fix the issue by
disallowing backward scan flag to be set when a query is in parallel
mode. This issue occurs during the implementation of full text search in
Babelfish. Fixing the issue unblocks full text search implementation.

Task: BABEL-4261, BABEL-4281
Authored-by: Robin Li <[email protected]>
Signed-off-by: Rohit Bhagat <[email protected]>
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