cloudspannerecosystem / yo

yo is a command-line tool to generate Go code for Google Cloud Spanner.
MIT License
312 stars 52 forks source link

add --ignore-unsupported-statements flag #116

Closed nktks closed 9 months ago

nktks commented 1 year ago

WHAT

Context

When user manages several DDLs in one schema file, user should filter DDLs to pass yo generate which yo doesn't support. This makes it difficult for user to use DDLs that yo does not support.

--ignore-unsupported-statements flag enables yo to skip these statements, and continue to generate codes. And if the user really made a mistake in supported syntax in DDL such as CREATE TABLE, CREATE INDEX, ALTER TABLE, this case still failed. Therefore, I don't think adding this flag will make it easy for users to make problems. How do you think?

ref: https://github.com/cloudspannerecosystem/wrench/issues/83

I agreeed CLA.

Thank you!

kazegusuri commented 1 year ago

It seems the improvement of statements separation is good but it's the same problem I commented in the issue in the wrench repo. For the feature for ignoring unsupported statements, I'm not sure is useful or not. If we can ignore the statements, we have another problem that we cannot use the tables in yo.

nktks commented 1 year ago

It seems the improvement of statements separation is good but it's the same problem I commented in the issue in the wrench repo.

I think so too. It will be good to have separating stmt implementation in spansql or something other place like under cloudspannerechosystem.

For the feature for ignoring unsupported statements, I'm not sure is useful or not.

I think, if user has below file:

CREATE ROLE hoge; -- want to use fgac role, doesn't supported yo yet.
CREATE CHANGE STREAM All FOR ALL; -- want to use change stream, doesn't supported yo yet.
CREATE TABLE A (id INT64 NOT NULL,) PRIMARY KEY(id); -- want to use yo for code generation

currently, yo generate will fail, and with --ignore-unsupported-statements enables to generate code related table A.

If we can ignore the statements, we have another problem that we cannot use the tables in yo.

If we use new feature in CREATE TABLE DDL which yo(spansql) doesn't support, yo generate still fail same as current version, so I think another problem will not occur.

Or do you mean another problem occurs?

nktks commented 1 year ago

I modified code:

nktks commented 9 months ago

I close this ticket. Instead I'll create another PR to skip CREATE CHANGE STREAM statements.