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: Introduce sqllogictest #152

Closed
wants to merge 12 commits into from

Conversation

Weijun-H
Copy link
Contributor

@Weijun-H Weijun-H commented Oct 13, 2024

Ticket(s) Closed

What

Why

How

  • introduce sqllogictest
  • convert some existing tests to .slt
  • Add CI to trigger sqllogictest
  • Add README about how to use sqllogictest

Tests

Comment on lines 18 to 20
// TECH DEBT: This file is a copy of the `db.rs` file from https://github.com/paradedb/paradedb/blob/dev/shared/src/fixtures/db.rs
// We duplicated because the paradedb repo may use a different version of pgrx than pg_analytics, but eventually we should
// move this into a separate crate without any dependencies on pgrx.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We an probably remove this, we don't plan to have different versions

Copy link
Collaborator

@philippemnoel philippemnoel left a comment

Choose a reason for hiding this comment

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

Could you please explain what sqllogictest does better than our existing test suite? Is it a replacement, or an addition to our current test suite?

This all looks quite complicated and a lot more code to maintain. What is missing from our current test suite that justifies this?

@Weijun-H
Copy link
Contributor Author

Could you please explain what sqllogictest does better than our existing test suite? Is it a replacement, or an addition to our current test suite?

This all looks quite complicated and a lot more code to maintain. What is missing from our current test suite that justifies this?

I think sqllogictest is a better addition to the current test suite. Because

  1. Ease of maintenance: As projects grow, test suites often become more complex and harder to maintain. Sqllogictest's standardized format can simplify the process of adding new tests and updating existing ones, potentially reducing the maintenance burden.
  2. Scalability: For larger projects with numerous complex SQL queries, sqllogictest's approach may be more manageable than traditional unit tests. Its design is geared towards handling a large volume of SQL-specific tests efficiently.
  3. Industry adoption: The fact that other database projects like DuckDB, Datafusion, Databend, and RisingWave use sqllogictest is a strong endorsement of its value for SQL-heavy applications. This widespread adoption suggests that it's a proven solution for SQL testing at scale.
  4. Complementary approach: Using sqllogictest alongside your current rstest suite can provide a more comprehensive testing strategy. Rstest can continue to handle unit tests and specific Rust-related scenarios, while sqllogictest focuses on SQL integration testing.
  5. Gradual adoption: You can start by implementing sqllogictest for new features or particularly complex areas of your codebase, gradually expanding its use as you see benefits, without immediately replacing your entire existing test suite.

Additionally, after #107 is completed, our sqllogictest test suite could use COPY TO to test by customized foreign tables.

For example

statement ok
COPY (SELECT 42 AS a, 'hello' AS b) TO 'query.parquet' (FORMAT PARQUET);

statement ok
CREATE FOREIGN DATA WRAPPER parquet_wrapper
HANDLER parquet_fdw_handler VALIDATOR parquet_fdw_validator;

statement ok
CREATE SERVER parquet_server FOREIGN DATA WRAPPER parquet_wrapper;

statement ok
CREATE FOREIGN TABLE trips ()
SERVER parquet_server
OPTIONS (files 'test_files/scratch/query.parquet');

# specific testing
statement ok
CREATE VIEW query_view AS
SELECT * FROM query_table;

......

@philippemnoel
Copy link
Collaborator

Could you please explain what sqllogictest does better than our existing test suite? Is it a replacement, or an addition to our current test suite?
This all looks quite complicated and a lot more code to maintain. What is missing from our current test suite that justifies this?

I think sqllogictest is a better addition to the current test suite. Because

  1. Ease of maintenance: As projects grow, test suites often become more complex and harder to maintain. Sqllogictest's standardized format can simplify the process of adding new tests and updating existing ones, potentially reducing the maintenance burden.
  2. Scalability: For larger projects with numerous complex SQL queries, sqllogictest's approach may be more manageable than traditional unit tests. Its design is geared towards handling a large volume of SQL-specific tests efficiently.
  3. Industry adoption: The fact that other database projects like DuckDB, Datafusion, Databend, and RisingWave use sqllogictest is a strong endorsement of its value for SQL-heavy applications. This widespread adoption suggests that it's a proven solution for SQL testing at scale.
  4. Complementary approach: Using sqllogictest alongside your current rstest suite can provide a more comprehensive testing strategy. Rstest can continue to handle unit tests and specific Rust-related scenarios, while sqllogictest focuses on SQL integration testing.
  5. Gradual adoption: You can start by implementing sqllogictest for new features or particularly complex areas of your codebase, gradually expanding its use as you see benefits, without immediately replacing your entire existing test suite.

Additionally, after #107 is completed, our sqllogictest test suite could use COPY TO to test by customized foreign tables.

For example

statement ok
COPY (SELECT 42 AS a, 'hello' AS b) TO 'query.parquet' (FORMAT PARQUET);

statement ok
CREATE FOREIGN DATA WRAPPER parquet_wrapper
HANDLER parquet_fdw_handler VALIDATOR parquet_fdw_validator;

statement ok
CREATE SERVER parquet_server FOREIGN DATA WRAPPER parquet_wrapper;

statement ok
CREATE FOREIGN TABLE trips ()
SERVER parquet_server
OPTIONS (files 'test_files/scratch/query.parquet');

# specific testing
statement ok
CREATE VIEW query_view AS
SELECT * FROM query_table;

......

Got it. That's convincing. Alright, then! I'm in favour of it, especially after splitting our tests into their own folder/crate. Thank you for doing this and for pushing for it!

@Weijun-H Weijun-H changed the title feat: Introduce sqllogictest(WIP) feat: Introduce sqllogictest Oct 26, 2024
@philippemnoel
Copy link
Collaborator

At this time, we've decided this is not a priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate sqllogictest
2 participants