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

spannertest: add support for row deletion policy #4782

Closed
alethenorio opened this issue Sep 21, 2021 · 4 comments · Fixed by #4961
Closed

spannertest: add support for row deletion policy #4782

alethenorio opened this issue Sep 21, 2021 · 4 comments · Fixed by #4961
Assignees
Labels
api: spanner Issues related to the Cloud Spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@alethenorio
Copy link

alethenorio commented Sep 21, 2021

Is your feature request related to a problem? Please describe.
We use spannertest to build an inmemory database based on a set of DDLs however as spannertest does not have any support for row deletion policy, it will fail calls to UpdateDDL with unhandled DDL table alteration type spansql.AddRowDeletionPolicy.

Describe the solution you'd like
The simplest solution would be to allow row deletion policy DDLs to not fail UpdateDDL calls (just ignore the row deletion policy) which would allow for performing basic functionality such as querying and other update

Describe alternatives you've considered
Alternative solutions could be to add actual support for row deletion policy (although given that the minimum is 1 day I am not sure how useful it is given the scope of spannertest). A configurable option allowing the user to choose to ignore row deletion policy and not fail UpdateDDL calls could also be an option.

Additional context

@alethenorio alethenorio added the triage me I really want to be triaged. label Sep 21, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Cloud Spanner API. label Sep 21, 2021
@hengfengli hengfengli assigned olavloite and unassigned hengfengli Sep 21, 2021
@hengfengli
Copy link
Contributor

hengfengli commented Sep 21, 2021

@olavloite Can you please triage this issue? Thanks.

@olavloite olavloite added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Sep 28, 2021
@olavloite
Copy link
Contributor

olavloite commented Sep 29, 2021

@alethenorio Do you have an example of a DDL statement with a row deletion policy that does not work for you? I had a second look into this today, and it turns out that it is implemented:

  1. Parsing:
    if p.eat(",", "ROW", "DELETION", "POLICY") {
    rdp, err := p.parseRowDeletionPolicy()
    if err != nil {
    return nil, err
    }
    ct.RowDeletionPolicy = &rdp
    }
  2. Tests for both creating and dropping:
    -- Table with row deletion policy.
    CREATE TABLE WithRowDeletionPolicy (
    Name STRING(MAX) NOT NULL,
    DelTimestamp TIMESTAMP NOT NULL,
    ) PRIMARY KEY (Name)
    , ROW DELETION POLICY ( OLDER_THAN ( DelTimestamp, INTERVAL 30 DAY ));
    ALTER TABLE WithRowDeletionPolicy DROP ROW DELETION POLICY;
    ALTER TABLE WithRowDeletionPolicy ADD ROW DELETION POLICY ( OLDER_THAN ( DelTimestamp, INTERVAL 30 DAY ));
    ALTER TABLE WithRowDeletionPolicy REPLACE ROW DELETION POLICY ( OLDER_THAN ( DelTimestamp, INTERVAL 30 DAY ));

@olavloite olavloite added type: question Request for information or clarification. Not an issue. and removed type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Sep 29, 2021
@olavloite
Copy link
Contributor

olavloite commented Oct 6, 2021

@alethenorio Friendly ping. Do you have an example of a statement with a row deletion policy that does not work?

@alethenorio
Copy link
Author

alethenorio commented Oct 6, 2021

@olavloite Thanks for the ping. Missed the first time around.

The issue is when calling this API

func (s *Server) UpdateDDL(ddl *spansql.DDL) error {

Which eventually ends up in this switch statement

switch stmt := stmt.(type) {

Which has a separate logic from spansql and does not support row deletion policies and instead return the default case of the switch.

@olavloite olavloite added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. labels Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Cloud Spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants