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

fix(frontend): make WITH optional for user create and alter #4414

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

yezizp2012
Copy link
Member

@yezizp2012 yezizp2012 commented Aug 3, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

As title. Pointed out by @CharlieSYH , we should keep compatibility with PostgreSQL.

I tested this statement and it retuned an error "ERROR: sql parser error: Expected end of statement, found: PASSWORD":
CREATE USER u1 PASSWORD 's1234';
But this works: CREATE USER u1 WITH PASSWORD 's1234';

Seems that 'with' is not optional. Can you confirm on that?

Originally posted by @CharlieSYH in #3074 (comment)

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove those that do not apply.

  • SQL commands, functions, and operators

Release note

Please create a release note for your changes. In the release note, focus on the impact on users, and mention the environment or conditions where the impact may occur.

Refer to a related PR or issue link (optional)

@yezizp2012 yezizp2012 requested a review from cnissnzg August 3, 2022 12:51
@github-actions github-actions bot added the type/fix Bug fix label Aug 3, 2022
@yezizp2012 yezizp2012 changed the title fix: make WITH optional for user create and alter fix(frontend): make WITH optional for user create and alter Aug 3, 2022
@yezizp2012 yezizp2012 requested review from xiangjinwu and removed request for BugenZhao August 3, 2022 12:56
@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #4414 (c92b6e8) into main (f103daf) will decrease coverage by 0.00%.
The diff coverage is 58.62%.

@@            Coverage Diff             @@
##             main    #4414      +/-   ##
==========================================
- Coverage   74.56%   74.56%   -0.01%     
==========================================
  Files         848      848              
  Lines      124163   124162       -1     
==========================================
- Hits        92582    92580       -2     
- Misses      31581    31582       +1     
Flag Coverage Δ
rust 74.56% <58.62%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/sqlparser/src/ast/statement.rs 83.65% <58.62%> (-0.07%) ⬇️
src/meta/src/manager/id.rs 94.56% <0.00%> (-0.55%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@CharlieSYH
Copy link
Contributor

CharlieSYH commented Aug 4, 2022

@yezizp2012
There is another behavior that differs from PG's:
In the current version of RW, password authentication is absent (login without password) during connection for user accounts created with PASSWORD NULL or PASSWORD '', or without the PASSWORD option. But PG treats empty string and null password differently:

If no password is specified, the password will be set to null and password authentication will always fail for that user. A null password can optionally be written explicitly as PASSWORD NULL. (https://www.postgresql.org/docs/current/sql-createrole.html)

In addition, is there any restrictions on the password?

@yezizp2012
Copy link
Member Author

@yezizp2012 There is another behavior that differs from PG's: In the current version of RW, password authentication is absent (login without password) during connection for user accounts created with PASSWORD NULL or PASSWORD '', or without the PASSWORD option. But PG treats empty string and null password differently:

If no password is specified, the password will be set to null and password authentication will always fail for that user. A null password can optionally be written explicitly as PASSWORD NULL. (https://www.postgresql.org/docs/current/sql-createrole.html)

In addition, is there any restrictions on the password?

Yes, in current implementation of RW, PASSWORD NULL or PASSWORD '' will be treated exactly the same and user created in that way will login successfully without providing any password. Well, we'd better to keep compatibility with PG here. @cnissnzg would you please to take a look and fix this?

Copy link
Contributor

@cnissnzg cnissnzg left a comment

Choose a reason for hiding this comment

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

LGTM

@CharlieSYH CharlieSYH added the user-facing-changes Contains changes that are visible to users label Aug 4, 2022
@mergify mergify bot merged commit ae0388a into main Aug 4, 2022
@mergify mergify bot deleted the fix/user-with-optional branch August 4, 2022 07:15
Little-Wallace added a commit to Little-Wallace/risingwave that referenced this pull request Aug 5, 2022
commit 051cabf
Author: Bohan Zhang <[email protected]>
Date:   Fri Aug 5 15:47:13 2022 +0800

    refactor(sink): rename sink properties (risingwavelabs#4465)

    * rename sink properties

    Signed-off-by: tabVersion <[email protected]>

    * fix

    Signed-off-by: tabVersion <[email protected]>

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit a12a9a8
Author: Noel Kwan <[email protected]>
Date:   Fri Aug 5 15:08:10 2022 +0800

    feat(sqlsmith): generate extreme values (risingwavelabs#4345)

    * gen extreme for integrals

    * test parsing output

    * workaround

    * use extreme for float, temporal

    * Revert "test parsing output"

    This reverts commit 3d12856cb847727f4009aa6ce8f6033cc4d24393.

    * use i64 for min/max float

    * interim

    * mention workarounds

    * clean

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit ecdadce
Author: jon-chuang <[email protected]>
Date:   Fri Aug 5 14:56:16 2022 +0800

    fix(planner): Tumble can accept CTE as input (risingwavelabs#4450)

    fix tumble window

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit a232f2e
Author: congyi <[email protected]>
Date:   Fri Aug 5 14:40:44 2022 +0800

    refactor(row-serde): make row-serde directory clearer and update comments (risingwavelabs#4443)

    * refactor row-serde and update comments

    * rename

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 9bd5db3
Author: Kexiang Wang <[email protected]>
Date:   Fri Aug 5 02:05:29 2022 -0400

    fix(meta): the ddl lock is not really locked (risingwavelabs#4461)

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 56a1897
Author: Richard Chien <[email protected]>
Date:   Fri Aug 5 13:03:26 2022 +0800

    feat(optimizer): support stateless 2-phase agg optimization for append-only min/max (risingwavelabs#4433)

    * use stateless 2-phase agg optimization for append-only min/max aggs

    * update plan snapshots

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 1ae6836
Author: sinemora <[email protected]>
Date:   Fri Aug 5 12:50:34 2022 +0800

    feat(frontend): add CREATEUSER and NOCREATEUSER option for user create/alter (risingwavelabs#4447)

    * initial

    * fix judge

    * fix

    * fix

    * fix pg_user

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit fc6b36d
Author: Liang <[email protected]>
Date:   Fri Aug 5 12:37:40 2022 +0800

    feat(meta): meta push initial hummock version into CN (risingwavelabs#4459)

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 80beff0
Author: jon-chuang <[email protected]>
Date:   Fri Aug 5 12:25:08 2022 +0800

    fix(planner): DynamicFilter's LHS should follow upstream distribution (risingwavelabs#4452)

    fix

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit ce17661
Author: Kexiang Wang <[email protected]>
Date:   Fri Aug 5 00:11:47 2022 -0400

    feat(meta): add source info and stream source split info in get_clust… (risingwavelabs#4277)

    feat(meta): add source info and stream source split info in get_cluster_info

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit f97a836
Author: jon-chuang <[email protected]>
Date:   Fri Aug 5 11:58:34 2022 +0800

    fix(planner): Pullup predicates in `LogicalScan` into `BatchLookupJoin`  (risingwavelabs#4453)

    fix

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 9b259bc
Author: zwang28 <[email protected]>
Date:   Fri Aug 5 11:13:33 2022 +0800

    feat(meta): piggyback extra info in heartbeat RPC. (risingwavelabs#4435)

    * feat(meta): piggyback extra info in heartbeat RPC.

    * collect extra_info_sources before start heartbeat task

commit bf1eb44
Author: Croxx <[email protected]>
Date:   Thu Aug 4 18:53:42 2022 +0800

    fix: build on non-linux target (risingwavelabs#4448)

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit fe9cc4d
Author: Wallace <[email protected]>
Date:   Thu Aug 4 18:40:30 2022 +0800

    fix(cache):  clean pending request immediately when future cancel (risingwavelabs#4422)

    * fix future drop

    Signed-off-by: Little-Wallace <[email protected]>

    * clean pending request

    Signed-off-by: Little-Wallace <[email protected]>

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit b016a50
Author: zwang28 <[email protected]>
Date:   Thu Aug 4 18:27:22 2022 +0800

    refactor(common): add rpc client pool (risingwavelabs#4410)

    refactor(common): add rpc client pool trait

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 934350c
Author: Huangjw <[email protected]>
Date:   Thu Aug 4 18:14:46 2022 +0800

    fix(ci): fix main ci build timeout (risingwavelabs#4444)

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 96c8ea6
Author: ZENOTME <[email protected]>
Date:   Thu Aug 4 18:02:13 2022 +0800

    chore: support more query statement in extended query mode (risingwavelabs#4441)

    * * support more query statement('select','describe','values','show','with')
    * support returning error when can't find a statement or portal  rather than panic
    * modify some filed name of message

    * * add comment
    * unify some name

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 2f98f9c
Author: Croxx <[email protected]>
Date:   Thu Aug 4 17:49:32 2022 +0800

    feat(cache): introduce tiered cache abstraction (risingwavelabs#4406)

    * feat(cache): introduce tiered cache abstraction

    * refactor write batch design

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 314473c
Author: Runji Wang <[email protected]>
Date:   Thu Aug 4 17:32:21 2022 +0800

    feat(test): make simulation test deterministic! (risingwavelabs#4336)

    * 3 compute nodes

    Signed-off-by: Runji Wang <[email protected]>

    * revert `madsim::time::Instant` to std's

    Signed-off-by: Runji Wang <[email protected]>

    * make `RowIdGenerator` async

    Signed-off-by: Runji Wang <[email protected]>

    * switch to nextest

    Signed-off-by: Runji Wang <[email protected]>

    * fix simulated e2e test

    Signed-off-by: Runji Wang <[email protected]>

    * update madsim to v0.2.0-alpha.6 and tonic to v0.8

    now additional system protoc is required.

    Signed-off-by: Runji Wang <[email protected]>

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 2d8ef9e
Author: August <[email protected]>
Date:   Thu Aug 4 17:10:26 2022 +0800

    fix: fix observer version check and add sync in compute/compactor node (risingwavelabs#4439)

    * fix: fix observer version check for init notification

    * add sync

    * fix sqlsmith

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit aff3352
Author: Li0k <[email protected]>
Date:   Thu Aug 4 16:57:32 2022 +0800

    chore(risedev): unify clippy between risedev c and pre-unit-test.sh (risingwavelabs#4436)

    * chore(risedev): unify clippy between risedev c and pre-unit-test.sh

    * fix(frontend): fix clippy

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit bffc66b
Author: Huangjw <[email protected]>
Date:   Thu Aug 4 16:45:08 2022 +0800

    chore(ci): remove repeated ci steps (risingwavelabs#4423)

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 7a0a99d
Author: xxchan <[email protected]>
Date:   Thu Aug 4 11:32:21 2022 +0300

    chore: update comments for dispatcher (risingwavelabs#4413)

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 667ab5d
Author: Bohan Zhang <[email protected]>
Date:   Thu Aug 4 16:19:20 2022 +0800

    feat: support explain sink (risingwavelabs#4430)

    * support explain sink

    Signed-off-by: tabVersion <[email protected]>

    * format

    Signed-off-by: tabVersion <[email protected]>

    * add ci for explain sink

    Signed-off-by: tabVersion <[email protected]>

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 5fe991c
Author: xxchan <[email protected]>
Date:   Thu Aug 4 11:06:50 2022 +0300

    fix: use SomeShard as the distribution of batch scan (risingwavelabs#4420)

    * fix: use SomeShard as the distribution of batch scan

    * drop

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 2d42495
Author: Li0k <[email protected]>
Date:   Thu Aug 4 15:29:15 2022 +0800

    fix(frontend): fix table option for internal table (risingwavelabs#4416)

    * feat(storage): save properties in ctx and remove support for source

    * feat(frontend): support properties for agg_plan and hash_join_plan

    * fix(frontend): assign properties for stream_materialize_view::create and add some limit

    * chore(front): remove redundant code

    * fix(frontend): fix clippy

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit ae0388a
Author: August <[email protected]>
Date:   Thu Aug 4 15:15:52 2022 +0800

    fix(frontend): make WITH optional for user create and alter (risingwavelabs#4414)

    fix: make WITH optional for user create and alter

    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit f103daf
Author: xiangjinwu <[email protected]>
Date:   Thu Aug 4 14:59:50 2022 +0800

    fix(binder): allow `Clone` for `expr::Subquery` as part of CTE (risingwavelabs#4424)

    fix(binder): allow Clone for expr::Subquery as part of CTE

commit ea36e52
Author: Lee Zong Yu <[email protected]>
Date:   Thu Aug 4 13:02:45 2022 +0800

    feat(sqlsmith): Enable gen_agg but workaround distinct agg (risingwavelabs#4421)

    Enable gen_agg

commit cc4224d
Author: Noel Kwan <[email protected]>
Date:   Thu Aug 4 11:44:17 2022 +0800

    feat(sqlsmith): generate explicit type castings (risingwavelabs#4419)

    * gen cast map

    * gen cast

    * dump cast table

    * gen explicit cast expr

    * avoid varchar casts

    * fix review comments

Signed-off-by: Little-Wallace <[email protected]>
nasnoisaac pushed a commit to nasnoisaac/risingwave that referenced this pull request Aug 9, 2022
…velabs#4414)

fix: make WITH optional for user create and alter

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/fix Bug fix user-facing-changes Contains changes that are visible to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants