-
Notifications
You must be signed in to change notification settings - Fork 22
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
Show planner error for SELECT INTO with DML in CTE #964
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we can move the check to the analysis stage in the transformCreateTableAsStmt
function. It may also make sense to make the error message more understandable to the user. Something like: "CREATE TABLE AS must not use data-modifying statements in WITH"
This comment was marked as resolved.
This comment was marked as resolved.
@whitehawk , @KnightMurloc
Probably merge 944 before and use the same walker here. It means that porting original patch now doesn't make sence and all kinds of refractoring are appropriate. I suggest considering complex check like its done in #944 . |
Isn't it a bug that if we don't have references to a modifying CTE, then we don't perform such a cte at all? create table t2 as with cte1 as (
insert into t values(1) returning *
),
cte2 as (
select * from unnest(
array(
select * from cte1
)
)
) select * from cte2;
|
If it's not refernced, it is not inlined and not executed. GP differs from PG in this situation cause GP does not have CteScan node
There is another case when cte has locus Replicated and there is also possibility to execute it with CTAS. However, if the main approach is throwing an error, making exclusions in this logic will look weird. I doubt that user will need this type of query at all. All such cases should be resolved by allowing executing several writing gangs, not here. |
This comment was marked as resolved.
This comment was marked as resolved.
Yes, it is done so according to comment #964 (comment) . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current solution also throw error on the plans in which DML is executed in InitPlan, which, as I understood from the discussion, we want to keep.
Show planner error for SELECT INTO with DML in CTE
Problem:
Greenplum fails to execute SELECT INTO and CREATE TABLE AS statements, whose
queries contain modifying CTEs, because Greenplum cannot have two writer
segworker groups for one query. In current implementation all the plans with
modifying CTE contain motion nodes, which separate slice with DML operation
from the slice, where the new table is created. And during execution stage an
error is thrown, telling that node with DML operation cannot be executed,
because one writer segworker already exists. However, showing the error during
planning stage would be more effective, therefore this commit proposes the
display of the error during planning.
This commit counts the number of writing slices during the plan tree traversal.
If there are more than one writing gang, it throws an error. In case writing
slice is found in the InitPlan, it is counted separately, as InitPlans are
executed independently.
Tests are partially taken from commit: cc2e30f
Co-authored-by: Alexander Kondakov a.kondakov@arenadata.io