Add IVM restrictions

[thoshiai]

Add following restrictions:

• DISTINCT ON
• TABLESAMPLE parameter
• inheritance parent table
• window function
• some aggregate options(such as FILTER, DISTINCT, ORDER and GROUPING SETS)
• targetlist containing IVM column
• simple subquery is only supported
• FOR UPDATE/SHARE
• empty target list
• UNION/INTERSECT/EXCEPT
• GROUPING SETS clauses

[yugo-n]

Although I commented for error messages, I'm wondering we want a some unified format for these messages. For example,

Materialized views containing DISTINCT ON are not incrementally maintainable.

like view_query_is_auto_updatable().

Any thoughts?

[yugo-n]

By the way, during this review, I found the following comment

065                 /* There is a possibility that we don't need to return an error */
1066                 if (qry->sortClause != NIL)
1067                     ereport(ERROR, (errmsg("ORDER BY clause is not supported with IVM")));

but it is ambiguous which code is explained by it and why, so this is a bit confusable. I think we need additional explanation and/or some while space or new line to make this easy to understand its sense. (You know, this is not a big issue....)

Rewriting this as below might work:

 if (qry->sortClause != NIL)   /* There is a possibility that we don't need to return an error */

Any other thoughts?

[thoshiai]

 if (qry->sortClause != NIL)   /* There is a possibility that we don't need to return an error */

Any other thoughts?

It sounds good! I think that other comments sould emulate it. First, I will fix this comment.

[yugo-n]

 if (qry->sortClause != NIL)   /* There is a possibility that we don't need to return an error */

Any other thoughts?

It sounds good! I think that other comments sould emulate it. First, I will fix this comment.

Well.. what are "other comments"?

[thoshiai]

Although I commented for error messages, I'm wondering we want a some unified format for these messages. For example,

Materialized views containing DISTINCT ON are not incrementally maintainable.

like view_query_is_auto_updatable().

Any thoughts?

I often see the error message syntax below : DISTINCT ON clause not allowed for incrementally maintainable materialized view or DISTINCT ON clause it not supported on incrementally maintainable materialized view

In addition, I think that this could be better if we add errcode(ERRCODE_SYNTAX_ERROR). For example:

ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("error message)));

[thoshiai]

It sounds good! I think that other comments sould emulate it. First, I will fix this comment.

Well.. what are "other comments"?

Maybe, (IVM) source code have confusable others like this , because we was adding the processing step by step.

[thoshiai]

I fixed for some resolved comments.

[yugo-n]

It sounds good! I think that other comments sould emulate it. First, I will fix this comment.

Well.. what are "other comments"?

Maybe, (IVM) source code have confusable others like this , because we was adding the processing step by step.

Maybe.... But, adding one liner comment for every step would not seem so good manner. (If we have to do this, our coding itself might not be good...)

For now, where I am concerning and want to fix is only this place at least as to query checking.

[yugo-n]

Although I commented for error messages, I'm wondering we want a some unified format for these messages. For example,

Materialized views containing DISTINCT ON are not incrementally maintainable.

like view_query_is_auto_updatable(). Any thoughts?

I often see the error message syntax below : DISTINCT ON clause not allowed for incrementally maintainable materialized view or DISTINCT ON clause it not supported on incrementally maintainable materialized view

Year, these also look good!

In addition, I think that this could be better if we add errcode(ERRCODE_SYNTAX_ERROR). For example:

ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("error message)));

I don't agree because the query check is not based on "syntax". If we use something, errcode(ERRCODE_FEATURE_NOT_SUPPORTED) would be suitable.

I think we can move the discussion above to other new issue.

[thoshiai]

Although I commented for error messages, I'm wondering we want a some unified format for these messages. For example,

Materialized views containing DISTINCT ON are not incrementally maintainable.

like view_query_is_auto_updatable(). Any thoughts?

I often see the error message syntax below : DISTINCT ON clause not allowed for incrementally maintainable materialized view or DISTINCT ON clause it not supported on incrementally maintainable materialized view

Year, these also look good!

In addition, I think that this could be better if we add errcode(ERRCODE_SYNTAX_ERROR). For example: ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("error message)));

I don't agree because the query check is not based on "syntax". If we use something, errcode(ERRCODE_FEATURE_NOT_SUPPORTED) would be suitable.

I think we can move the discussion above to other new issue.

Thank you for your comments. I add this in issues.

[thoshiai]

I fixed odd review comments.

[yugo-n]

I fixed odd review comments.

Thanks. It seems good. I'll merge it.