[question] About parse 'ALTER TABLE' in postgreSQL

Kijin-Seija commented 9 months ago

Version latest

The Type of SQL PostgreSQL

Your Code

class MyVisitor extends AbstractParseTreeVisitor<void> implements PostgreSQLParserVisitor<void> {
  visitAltertablestmt(ctx: any) {
    console.log('visitAltertablestmt', ctx)
  }
  visitAlterdatabasestmt(ctx: any) {
    console.log('visitAlterdatabasestmt', ctx)
  }
  ...
}

const visitor = new MyVisitor()

const tree1 = parser.parse('ALTER TABLE table1')
visitor.visit(tree1) // cannot print 'visitAltertablestmt'

const tree2 = parser.parse('ALTER TABLE table1 DROP column1')
visitor.visit(tree2) // ok, can print 'visitAltertablestmt'

const tree3 = parse.parse('ALTER DATABASE db1')
visitor.visit(tree3) // ok, can print 'visitAlterdatabasestmt'

Problem

在postregreSQL的语法解析时，无法将不完整但有基本结构的ALTER TABLE语法识别为 Altertablestmt 节点。在对应的antlr4语法文件 https://github.com/DTStack/dt-sql-parser/blob/main/src/grammar/pgsql/PostgreSQLParser.g4 中可以看到，altertablestmt语法要求ALTER TABLE必须要有alter_table_cmds。这意味着最短可识别语句必须至少在tableName后面有一个关键字，比如ALTER TABLE table1 DROP 。如果仅为 ALTER TABLE table1则无法识别

In the syntax parsing of PostgreSQL, an incomplete but fundamentally structured ALTER TABLE syntax cannot be recognized as an Altertablestmt node. In the corresponding ANTLR4 grammar file: https://github.com/DTStack/dt-sql-parser/blob/main/src/grammar/pgsql/PostgreSQLParser.g4 it can be seen that the altertablestmt syntax requires ALTER TABLE to have at least one alter_table_cmds. This means that the shortest recognizable statement must have at least one keyword following the tableName, such as ALTER TABLE table1 DROP. If it is simply ALTER TABLE table1, it cannot be recognized.

altertablestmt
    : KW_ALTER KW_TABLE opt_if_exists? relation_expr (alter_table_cmds | partition_cmd)
    | KW_ALTER KW_TABLE KW_ALL KW_IN KW_TABLESPACE tablespace_name (KW_OWNED KW_BY role_list)? KW_SET KW_TABLESPACE tablespace_name_create opt_nowait?
    | KW_ALTER KW_TABLE opt_if_exists? table_name KW_ATTACH KW_PARTITION qualified_name (
        KW_FOR KW_VALUES partition_bound_spec
        | KW_DEFAULT
    )
    | KW_ALTER KW_TABLE opt_if_exists? table_name KW_DETACH KW_PARTITION qualified_name (
        KW_CONCURRENTLY
        | KW_FINALIZE
    )?
    | KW_ALTER KW_INDEX opt_if_exists? qualified_name (alter_table_cmds | index_partition_cmd)
    | KW_ALTER KW_INDEX KW_ALL KW_IN KW_TABLESPACE tablespace_name (KW_OWNED KW_BY role_list)? KW_SET KW_TABLESPACE tablespace_name_create opt_nowait?
    | KW_ALTER KW_SEQUENCE opt_if_exists? qualified_name alter_table_cmds
    | KW_ALTER KW_VIEW opt_if_exists? view_name alter_table_cmds
    | KW_ALTER KW_MATERIALIZED KW_VIEW opt_if_exists? view_name alter_table_cmds
    | KW_ALTER KW_MATERIALIZED KW_VIEW KW_ALL KW_IN KW_TABLESPACE tablespace_name (
        KW_OWNED KW_BY role_list
    )? KW_SET KW_TABLESPACE tablespace_name_create opt_nowait?
    | KW_ALTER KW_FOREIGN KW_TABLE opt_if_exists? relation_expr alter_table_cmds
    ;

但是，在ALTER DATABASE的语法里就没有这样的限制，只需要 ALTER DATABASE database_name就可以匹配Alterdatabasestmt。

However, in the syntax for ALTER DATABASE, there is no such restriction. Simply using ALTER DATABASE database_name is sufficient to match the Alterdatabasestmt.

alterdatabasestmt
    : KW_ALTER KW_DATABASE database_name (
        (KW_WITH? createdb_opt_list)?
        | createdb_opt_list?
        | (KW_SET KW_TABLESPACE tablespace_name_create)?
    )
    ;

我看了下antlr官方最新的语法也是如此，不知道这样设计的原理是什么。因为对postgreSQL的语法不是完全精通，所以想先提出来讨论一下，如果多数人觉得这里有疑问的话，我会再向https://github.com/antlr/grammars-v4 提issue p.s. 考虑到官方可能认为ALTER TABLE table不是一个可执行的语句，后面必须跟命令，我去翻了一下Mysql的Alterdatabasestmt, 发现Mysql应该是可以识别的（没有测试）

I checked the latest official ANTLR grammar and it's the same there; I'm not sure what the rationale behind this design is. Since I'm not fully proficient in PostgreSQL syntax, I wanted to bring this up for discussion first. If the majority think there's an issue here, I will consider raising an issue at https://github.com/antlr/grammars-v4.

p.s. Considering that the official source might regard ALTER TABLE table as not an executable statement and that it must be followed by a command, I looked into MySQL's Alterdatabasestmt. It seems that MySQL should be able to recognize it (though I haven't tested it).

alterTable
    : KW_ALTER KW_TABLE tableName (alterOption (',' alterOption)*)? (
        alterPartitionSpecification alterPartitionSpecification*
    )?
    ;

mumiao commented 9 months ago

@Cythia828 PgSQL的

Cythia828 commented 9 months ago

@Kijin-Seija 感谢你这边对于疑惑点细致的描述，针对ALTER TABLE table1不能作为最短合法语句去识别这一点，我们参照的是PostgreSQL的官方文档提供的语法文件，具体地址：https://www.postgresql.org/docs/16/sql-altertable.html 可以对照一下，如果有我们还未考虑到的地方欢迎再次提问探讨！

mumiao commented 9 months ago

Version latest

The Type of SQL PostgreSQL

Your Code
class MyVisitor extends AbstractParseTreeVisitor<void> implements PostgreSQLParserVisitor<void> {
  visitAltertablestmt(ctx: any) {
    console.log('visitAltertablestmt', ctx)
  }
  visitAlterdatabasestmt(ctx: any) {
    console.log('visitAlterdatabasestmt', ctx)
  }
  ...
}

const visitor = new MyVisitor()

const tree1 = parser.parse('ALTER TABLE table1')
visitor.visit(tree1) // cannot print 'visitAltertablestmt'

const tree2 = parser.parse('ALTER TABLE table1 DROP column1')
visitor.visit(tree2) // ok, can print 'visitAltertablestmt'

const tree3 = parse.parse('ALTER DATABASE db1')
visitor.visit(tree3) // ok, can print 'visitAlterdatabasestmt'
Problem

在postregreSQL的语法解析时，无法将不完整但有基本结构的ALTER TABLE语法识别为 Altertablestmt 节点。在对应的antlr4语法文件 https://github.com/DTStack/dt-sql-parser/blob/main/src/grammar/pgsql/PostgreSQLParser.g4 中可以看到，altertablestmt语法要求ALTER TABLE必须要有alter_table_cmds。这意味着最短可识别语句必须至少在tableName后面有一个关键字，比如ALTER TABLE table1 DROP 。如果仅为 ALTER TABLE table1则无法识别

In the syntax parsing of PostgreSQL, an incomplete but fundamentally structured ALTER TABLE syntax cannot be recognized as an Altertablestmt node. In the corresponding ANTLR4 grammar file: https://github.com/DTStack/dt-sql-parser/blob/main/src/grammar/pgsql/PostgreSQLParser.g4 it can be seen that the altertablestmt syntax requires ALTER TABLE to have at least one alter_table_cmds. This means that the shortest recognizable statement must have at least one keyword following the tableName, such as ALTER TABLE table1 DROP. If it is simply ALTER TABLE table1, it cannot be recognized.
altertablestmt
    : KW_ALTER KW_TABLE opt_if_exists? relation_expr (alter_table_cmds | partition_cmd)
    | KW_ALTER KW_TABLE KW_ALL KW_IN KW_TABLESPACE tablespace_name (KW_OWNED KW_BY role_list)? KW_SET KW_TABLESPACE tablespace_name_create opt_nowait?
    | KW_ALTER KW_TABLE opt_if_exists? table_name KW_ATTACH KW_PARTITION qualified_name (
        KW_FOR KW_VALUES partition_bound_spec
        | KW_DEFAULT
    )
    | KW_ALTER KW_TABLE opt_if_exists? table_name KW_DETACH KW_PARTITION qualified_name (
        KW_CONCURRENTLY
        | KW_FINALIZE
    )?
    | KW_ALTER KW_INDEX opt_if_exists? qualified_name (alter_table_cmds | index_partition_cmd)
    | KW_ALTER KW_INDEX KW_ALL KW_IN KW_TABLESPACE tablespace_name (KW_OWNED KW_BY role_list)? KW_SET KW_TABLESPACE tablespace_name_create opt_nowait?
    | KW_ALTER KW_SEQUENCE opt_if_exists? qualified_name alter_table_cmds
    | KW_ALTER KW_VIEW opt_if_exists? view_name alter_table_cmds
    | KW_ALTER KW_MATERIALIZED KW_VIEW opt_if_exists? view_name alter_table_cmds
    | KW_ALTER KW_MATERIALIZED KW_VIEW KW_ALL KW_IN KW_TABLESPACE tablespace_name (
        KW_OWNED KW_BY role_list
    )? KW_SET KW_TABLESPACE tablespace_name_create opt_nowait?
    | KW_ALTER KW_FOREIGN KW_TABLE opt_if_exists? relation_expr alter_table_cmds
    ;
但是，在ALTER DATABASE的语法里就没有这样的限制，只需要 ALTER DATABASE database_name就可以匹配Alterdatabasestmt。

However, in the syntax for ALTER DATABASE, there is no such restriction. Simply using ALTER DATABASE database_name is sufficient to match the Alterdatabasestmt.
alterdatabasestmt
   : KW_ALTER KW_DATABASE database_name (
       (KW_WITH? createdb_opt_list)?
       | createdb_opt_list?
       | (KW_SET KW_TABLESPACE tablespace_name_create)?
   )
   ;
我看了下antlr官方最新的语法也是如此，不知道这样设计的原理是什么。因为对postgreSQL的语法不是完全精通，所以想先提出来讨论一下，如果多数人觉得这里有疑问的话，我会再向https://github.com/antlr/grammars-v4 提issue p.s. 考虑到官方可能认为ALTER TABLE table不是一个可执行的语句，后面必须跟命令，我去翻了一下Mysql的Alterdatabasestmt, 发现Mysql应该是可以识别的（没有测试）

I checked the latest official ANTLR grammar and it's the same there; I'm not sure what the rationale behind this design is. Since I'm not fully proficient in PostgreSQL syntax, I wanted to bring this up for discussion first. If the majority think there's an issue here, I will consider raising an issue at https://github.com/antlr/grammars-v4.

p.s. Considering that the official source might regard ALTER TABLE table as not an executable statement and that it must be followed by a command, I looked into MySQL's Alterdatabasestmt. It seems that MySQL should be able to recognize it (though I haven't tested it).
alterTable
    : KW_ALTER KW_TABLE tableName (alterOption (',' alterOption)*)? (
        alterPartitionSpecification alterPartitionSpecification*
    )?
    ;

说下我的理解，Alter Table 是对于表结构的修改，必须包含要对表进行的具体操作，比如添加、修改或删除列等。因此，必须在 ALTER TABLE 后面指定具体的操作，例如添加列、修改列类型、删除列等，翻译成sql语句就是：Alter Table tablename drop xxx

Kijin-Seija commented 9 months ago

@Kijin-Seija 感谢你这边对于疑惑点细致的描述，针对ALTER TABLE table1不能作为最短合法语句去识别这一点，我们参照的是PostgreSQL的官方文档提供的语法文件，具体地址：https://www.postgresql.org/docs/16/sql-altertable.html 可以对照一下，如果有我们还未考虑到的地方欢迎再次提问探讨！

@Cythia828 @mumiao 感谢回复。根据你们的意思，我去查询了对应资料后，觉得这个问题可以作为一个discussion，当前的设计基于官方，也有其合理性。我会在此回复后自己关闭issue。以下是我这边的一些想法。

确实在postgres官方的文档中，action是必须的而在MySql文档中，action不是必须的

我这边的使用场景是代码编辑器开发。在这种情况下，用户可能在输入 ALTER TABLE之后就希望得到一些基于table的推荐。当前的suggestion推荐的也确实是table，但是我这边遇到的问题是，在大部分推荐table的地方，也可以给用户推荐view。但ALTER TABLE 不可以推荐view，只能推荐table。所以我需要一个标识来判断当前的语法是ALTER TABLE。基于目前的逻辑，我采用的workaround是用Mysql的parser把语句再解析一遍，然后合并两次解析的结果来确认是否是ALTER TABLE。就是方式比较绕:D。综上所述才提出了这个问题。不知道你们对于这个场景有没有什么推荐的方案呢？

Cythia828 commented 9 months ago

@Kijin-Seija 感谢提供你的想法和建议～可以看到，目前我们对于不同sql语句出现的tableName、viewName是进行了分类收集的，这样就保证了在给到suggestion的时候会准确的告诉你此处推荐是tableName还是viewName，或者说tableName、viewName都可以。所以目前我们内部支持的能力应该是可以对你的问题提供帮助的。

Kijin-Seija commented 9 months ago

@Cythia828 我的意思并不是区分ALTER TABLE 和 ALTER VIEW，而是如下情况：

考虑如下语句

SELECT * FROM |

在用户输入到这里的时候，dt-sql-parser推荐的补全类型是table和function

但实际上，这里我们也可以输入view，从官方的文档里也可以看到这一用法

经过我向数据库同事咨询，除了select from 以外，还有很多地方是即可以填table也可以填view的，因此我这边就添加了逻辑:当suggestion是table时，返回的推荐词列表里除了tableList，还会concat viewList.

采用了以上逻辑后，就需要考虑到例外情况，比如ALTER TABLE。因此产生了一开始说到的问题。

Cythia828 commented 9 months ago

@Kijin-Seija 感谢你这边对于疑惑点细致的描述，针对ALTER TABLE table1不能作为最短合法语句去识别这一点，我们参照的是PostgreSQL的官方文档提供的语法文件，具体地址：https://www.postgresql.org/docs/16/sql-altertable.html 可以对照一下，如果有我们还未考虑到的地方欢迎再次提问探讨！

@Cythia828 @mumiao 感谢回复。根据你们的意思，我去查询了对应资料后，觉得这个问题可以作为一个discussion，当前的设计基于官方，也有其合理性。我会在此回复后自己关闭issue。以下是我这边的一些想法。

确实在postgres官方的文档中，action是必须的而在MySql文档中，action不是必须的

我这边的使用场景是代码编辑器开发。在这种情况下，用户可能在输入 ALTER TABLE之后就希望得到一些基于table的推荐。当前的suggestion推荐的也确实是table，但是我这边遇到的问题是，在大部分推荐table的地方，也可以给用户推荐view。但ALTER TABLE 不可以推荐view，只能推荐table。所以我需要一个标识来判断当前的语法是ALTER TABLE。基于目前的逻辑，我采用的workaround是用Mysql的parser把语句再解析一遍，然后合并两次解析的结果来确认是否是ALTER TABLE。就是方式比较绕:D。综上所述才提出了这个问题。不知道你们对于这个场景有没有什么推荐的方案呢？

我这边理解你的描述意思是希望达到ALTER TABLE 不可以推荐view，只能推荐table这个效果是吧？我刚刚的回复意思是我们针对functionName、tableName等都做了收集，具体代码可参考：在每条语句中都是做了变量名替换的，是否会推荐view，还是只能推荐table，是根据语法来的，也就是说ALTER TABLE 不可以推荐view，只能推荐table，这是语法本身结构，所以我们只会推荐table啊

Cythia828 commented 9 months ago

@Cythia828 我的意思并不是区分ALTER TABLE 和 ALTER VIEW，而是如下情况：

考虑如下语句
SELECT * FROM |
在用户输入到这里的时候，dt-sql-parser推荐的补全类型是table和function

但实际上，这里我们也可以输入view，从官方的文档里也可以看到这一用法

经过我向数据库同事咨询，除了select from 以外，还有很多地方是即可以填table也可以填view的，因此我这边就添加了逻辑:当suggestion是table时，返回的推荐词列表里除了tableList，还会concat viewList.

采用了以上逻辑后，就需要考虑到例外情况，比如ALTER TABLE。因此产生了一开始说到的问题。

针对你这里的举例select from语句，可以参考pgsql官方文档哦，pgsql里select from后面只支持functionName和tableName https://www.postgresql.org/docs/current/sql-select.html 欢迎继续提问探讨哦～

Kijin-Seija commented 9 months ago

@Cythia828 这一点我不置可否，确实，官方文档里select语法并不支持后面跟viewName，但是同样是在官方文档里，view的示例代码里也确实存在select * from viewName这种用法 https://www.postgresql.org/docs/current/tutorial-views.html#TUTORIAL-VIEWS

根据以上讨论，其实这个问题的根源不在ALTER TABLE，对于alter table的parser和推荐处理这块没有问题。

根源在于SELECT FROM (以及其他一些类似的语法)后面是否应该推荐view，如果推荐了view，那么自然也不需要处理ALTER TABLE的问题了。

Cythia828 commented 9 months ago

@Kijin-Seija 确实，如你所说，我也在官方文档中找到了select * from viewName的用法，对于这个问题，既然发现了，且官方支持有迹可循，那我们这边会加上的，请持续关注我们的更新，感谢支持～

DTStack / dt-sql-parser

[question] About parse 'ALTER TABLE' in postgreSQL #254