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

feat(sqlsmith): test struct and list #5951

Merged
merged 44 commits into from
Jan 4, 2023
Merged

feat(sqlsmith): test struct and list #5951

merged 44 commits into from
Jan 4, 2023

Conversation

neverchanje
Copy link
Contributor

@neverchanje neverchanje commented Oct 20, 2022

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

What's changed and what's your intention?

Add more input types to sqlsmith (struct, list).

Motivation for refactoring DataTypeName

  • Previously we use DataTypeName internally to indicate which expressions to generate.
  • This is insufficient, since it elides struct and list internal types.
  • Now we use DataType directly.
  • In this PR we support generating these new variants as scalar values, and columns.
  • Other PR may support generating functions, see: sqlsmith: Generate struct and list exprs #7132.
    • This will likely work by generating some variants of structs and lists we can choose from,
    • During setup: insert function signatures which can work with these structs and lists.
    • During setup: Define relations with these variants as columns.

Misc

Checklist

  • Fix struct (name fields are missing) expected.
  • gen struct and list scalar values
  • 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

If your pull request contains user-facing changes, please specify the types of the changes, and create a release note. Otherwise, please feel free to remove this section.

Types of user-facing changes

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

  • Installation and deployment
  • Connector (sources & sinks)
  • SQL commands, functions, and operators
  • RisingWave cluster configuration changes
  • Other (please specify in the release note below)

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)

#7132

@neverchanje neverchanje changed the title (WIP) test: add random types test to sqlsmith test: add random types test to sqlsmith (WIP) Oct 20, 2022
@github-actions github-actions bot added component/test Test related issue. and removed Invalid PR Title labels Oct 20, 2022
@github-actions
Copy link
Contributor

This PR has been open for 60 days with no activity. Could you please update the status? Feel free to ping a reviewer if you are waiting for review.

@lmatz
Copy link
Contributor

lmatz commented Dec 20, 2022

Do you want to merge or do you want @kwannoel to take a look,
I guess the motivation is that we have more seed tables to start with.
@kwannoel, do you think the current sqlsmith cover all of the type tests introduced in this PR?

@kwannoel
Copy link
Contributor

kwannoel commented Dec 20, 2022

Do you want to merge or do you want @kwannoel to take a look, I guess the motivation is that we have more seed tables to start with. @kwannoel, do you think the current sqlsmith cover all of the type tests introduced in this PR?

What sort of type tests are introduced in particular? Or what tests are planned to be introduced in this PR? Currently just see more seed tables.

@lmatz
Copy link
Contributor

lmatz commented Dec 20, 2022

Do you want to merge or do you want @kwannoel to take a look, I guess the motivation is that we have more seed tables to start with. @kwannoel, do you think the current sqlsmith cover all of the type tests introduced in this PR?

What sort of type tests are introduced in particular?

CREATE TABLE t1 (
    c1 boolean,
    c2 smallint,
    c3 integer,
    c4 bigint,
    c5 real,
    c6 double precision,
    c7 numeric,
    c8 date,
    c9 varchar,
    c10 time without time zone,
    c11 timestamp without time zone,
    c12 timestamp with time zone,
    c13 interval,
    c14 struct < a integer >,
    c15 integer []
);

CREATE TABLE t2 (
    c1 boolean,
    c2 smallint,
    c3 integer,
    c4 bigint,
    c5 real,
    c6 double precision,
    c7 numeric,
    c8 date,
    c9 varchar,
    c10 time without time zone,
    c11 timestamp without time zone,
    c12 timestamp with time zone,
    c13 interval,
    c14 struct < a integer >,
    c15 integer []
);

because we can have all kinds of composite types and nested types, so only randomization can cover more of them I guess?

@kwannoel
Copy link
Contributor

kwannoel commented Dec 20, 2022

Do you want to merge or do you want @kwannoel to take a look, I guess the motivation is that we have more seed tables to start with. @kwannoel, do you think the current sqlsmith cover all of the type tests introduced in this PR?

What sort of type tests are introduced in particular?

CREATE TABLE t1 (
    c1 boolean,
    c2 smallint,
    c3 integer,
    c4 bigint,
    c5 real,
    c6 double precision,
    c7 numeric,
    c8 date,
    c9 varchar,
    c10 time without time zone,
    c11 timestamp without time zone,
    c12 timestamp with time zone,
    c13 interval,
    c14 struct < a integer >,
    c15 integer []
);

CREATE TABLE t2 (
    c1 boolean,
    c2 smallint,
    c3 integer,
    c4 bigint,
    c5 real,
    c6 double precision,
    c7 numeric,
    c8 date,
    c9 varchar,
    c10 time without time zone,
    c11 timestamp without time zone,
    c12 timestamp with time zone,
    c13 interval,
    c14 struct < a integer >,
    c15 integer []
);

because we can have all kinds of composite types and nested types, so only randomization can cover more of them I guess?

That makes sense. c14 and c15 will cover cases not currently covered by sqlsmith. I can take a look if @neverchanje if busy.

@kwannoel kwannoel self-assigned this Dec 20, 2022
@kwannoel kwannoel force-pushed the wt-sqlsmith branch 2 times, most recently from 7848318 to 1886a12 Compare December 30, 2022 14:38
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has totally checked 2595 files.

Valid Invalid Ignored Fixed
1244 1 1350 0
Click to see the invalid file list
  • src/tests/sqlsmith/src/sql_gen/types.rs

@@ -0,0 +1,148 @@
// Copyright 2022 Singularity Data
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Copyright 2022 Singularity Data
// Copyright 2023 Singularity Data
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// Copyright 2022 Singularity Data

@kwannoel kwannoel changed the title test: add random types test to sqlsmith (WIP) feat(sqlsmith): test struct and list Jan 3, 2023
@github-actions github-actions bot added type/feature and removed component/test Test related issue. labels Jan 3, 2023
# Conflicts:
#	src/tests/sqlsmith/src/sql_gen/scalar.rs
#	src/tests/sqlsmith/src/sql_gen/utils.rs
@kwannoel kwannoel requested a review from lmatz January 4, 2023 13:43
@mergify mergify bot merged commit 98ae6ce into main Jan 4, 2023
@mergify mergify bot deleted the wt-sqlsmith branch January 4, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants