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

ISSUE-4262: prohibits using reserved table option in create table statement. #4632

Merged
merged 7 commits into from
Mar 31, 2022

Conversation

dantengsky
Copy link
Member

@dantengsky dantengsky commented Mar 30, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

  • DATABASE_ID and SNAPSHOT_VER are now reserved fuse table option keys
  • they are not allowed to be specified in the CREATE TABLE statement
  • the values of those options are hidden, for the result of SHOW CREATE TABLE statement

MISC:

  • unify dev stateless test with the corresponding CI tests

  • ci-run-tests-embedded.sh does not enable table cache
    which is a better setting to discover more potential flaws

  • no longer use table option key SNAPSHOT_VER

  • infer the version of snapshot from snapshot location

    Only in the bootstrap phase, i.e. the version of snapshot that comes
    directly from TableInfo is deduced in this way.

    For the versions of history snapshots, the numeric values that kept
    in meta data file are used.

    Later, when zero-copy clone is implemented seriously, the above inconsistency
    should disappear.

Changelog

  • Bug Fix
  • Not for changelog (changelog entry is not required)

Related Issues

Fixes #4624

Test Plan

Unit Tests

Stateless Tests

@vercel
Copy link

vercel bot commented Mar 30, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/databend/databend/B4D1SNghysUsLGRxeikXL7RHTYR9
✅ Preview: Canceled

[Deployment for 8b29e62 canceled]

@mergify
Copy link
Contributor

mergify bot commented Mar 30, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@mergify mergify bot added pr-bugfix this PR patches a bug in codebase pr-not-for-changelog labels Mar 30, 2022
@dantengsky
Copy link
Member Author

@BohuTANG

Got a problem in the stateless test, make stateless-test gives positive, but ./scripts/ci/ci-run-tests-embedded-meta.sh negative.

Seems there are some flaws in the arrangement of snapshot versioning, digging into it.

PR will be marked as ready after the CI passed.

- unify dev stateless test with the corresponding CI tests
- ci-run-tests-embedded.sh does not enable table cache
  which is a better setting to discovery more potential flaws
- no longer use table option key SNAPSHOT_VER

- infer the version of snapshot from snapshot location

  Only in the bootstrap phase, i.e. the version of snapshot that comes
  directly from TableInfo is deduced in this way.

  For the versions of history snapshots, the numeric values that kept
  in meta data file are used.

  Later, when zero-copy clone is implemented seriously, the above inconsistency
  should disappear.
@dantengsky
Copy link
Member Author

dantengsky commented Mar 31, 2022

@BohuTANG PTAL:

besides issue #4624 that this PR is trying to address:

  • unify dev stateless test with the corresponding CI tests
    make stateless-test now uses ci-run-tests-embedded.sh
    which does not enable table cache, imo it is a more proper setting that may discover more potential flaws

  • no longer use table option key SNAPSHOT_VER

    instead, inferring the versions of snapshots from the snapshot's location

    Only in the bootstrap phase, i.e. the version of snapshot that comes
    directly from TableInfo is deduced in this way.

    For the versions of history snapshots, the numeric values that kept
    in meta data file are used.

    Later, when zero-copy clone is implemented seriously, the above inconsistency
    should disappear.

  • backward compatibility of this PR has also been verified manually in EC2 + S3

@dantengsky dantengsky marked this pull request as ready for review March 31, 2022 09:52
Copy link
Member

@BohuTANG BohuTANG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@BohuTANG BohuTANG merged commit 4bec66e into databendlabs:main Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-review pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show create table prints the fuse engine internal information
3 participants