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

Implement TEMPLATE interceptor #51

Closed
waynexia opened this issue Apr 17, 2023 · 13 comments · Fixed by #63
Closed

Implement TEMPLATE interceptor #51

waynexia opened this issue Apr 17, 2023 · 13 comments · Fixed by #63
Assignees
Labels
enhancement New feature or request

Comments

@waynexia
Copy link
Member

Describe This Problem

It's a bit frustrating when we want to repeat SQL several times. Like insert 100 rows

INSERT INTO t (c) VALUES
(1),
(2),
...
(100);

or query with all the aggregators

SELECT count(c) FROM t;
SELECT avg(c) FROM t;
...

Proposal

Implement a query interceptor that can do this for us. It can automatically replace variables from the given value set. It will looks like

-- SQLNESS TEMPLATE i=0..100
INSERT INTO t (c) VALUES {{i}};
-- SQLNESS TEMPLATE aggr={avg, count, stddev, stdvar, sum}
SELECT {{aggr}}(c) from t;

Additional Context

As @jiacai2050 mentioned in #50 (comment), crate minijinja may help to implement this

@waynexia waynexia added the enhancement New feature or request label Apr 17, 2023
@waynexia waynexia self-assigned this Apr 18, 2023
@jiacai2050
Copy link
Member

jiacai2050 commented Apr 18, 2023

This is a big feature, it's another advantage over sqllogictest.

We may need more document about interceptor after this is finished.

@jiacai2050
Copy link
Member

-- SQLNESS TEMPLATE aggr={avg, count, stddev, stdvar, sum}, secret=env:MY_TOKEN
SELECT {{aggr}}(c) from t where token={{secret}};

How about this syntax to read environment variable? or just $MY_TOKEN?

@waynexia
Copy link
Member Author

I still insist we need to separate TEMPLATE and ENV, as you write they have lots of differences:

  • env should not be rendered, at any condition. But template can
  • env's value isn't provided in the input file, while template's value set is part of the input
  • env and template values have different grammar to define

Anyway, let me first implement the consensus part, and put off the controversies

@jiacai2050
Copy link
Member

I'm fine with two interceptors, just think of ENV and TEMPLATE have similar 'backend' implementation, only the 'UI' part is different.

@tisonkun
Copy link

Cross ref #56.

Do we have plan to go forward TEMPLATE now?

@jiacai2050
Copy link
Member

I'm afraid no one is working on this.

@jiacai2050
Copy link
Member

I plan to work on it this week @tisonkun

@waynexia
Copy link
Member Author

Thanks. Do you have a rough UI-level design of this interceptor? This interceptor is expected to be more flexible and complex than others.

@jiacai2050
Copy link
Member

The syntax is minijinja, there is an online demo:

The demo above use JSON to populate data for the template, this maybe a little verbose, I plan to explore if TOML is supported.

For advanced usage like range replacement, minijinja support register functions to environment, and there is already one for us

Also we can implement our functions in future to enhance it.

@jiacai2050 jiacai2050 assigned jiacai2050 and unassigned waynexia Mar 15, 2024
@waynexia
Copy link
Member Author

I was investigating https://crates.io/crates/handlebars, the common problem with minijinja is we have to provide value and SQL templates separately. And they may be provided in either enumerate or range form. Wondering if you have some advice or examples.

Another undetermined problem is whether we should render the expanded query in .result file. The content might be overwhelming.

@jiacai2050
Copy link
Member

jiacai2050 commented Mar 15, 2024

I have some experience in minijinja, it works well for me, and couldn't get what problems are you mean.

Wondering if you have some advice or examples.

-- SQLNESS TEMPLATE

INSERT INTO t (c) VALUES
{% for num in range(1, 11) %}
  ({{ num }}),
{% endfor %}
;


-- SQLNESS TEMPLATE aggr={avg, count, stddev, stdvar, sum}

{%- for item in aggr %}
SELECT {{item}}(c) from t;
{%- endfor %}

This is what it looks like for demo in description.

Another undetermined problem is whether we should render the expanded query in .result file. The content might be overwhelming.

This sounds not a problem for me, if we ignore the rendered query, this will make it hard to debug, so we need to write them to result file.

@waynexia
Copy link
Member Author

I have some experience in minijinja, it works well for me, and couldn't get what problems are you mean.

Wondering if you have some advice or examples.

-- SQLNESS TEMPLATE

INSERT INTO t (c) VALUES
{% for num in range(1, 11) %}
  ({{ num }}),
{% endfor %}
;


-- SQLNESS TEMPLATE aggr={avg, count, stddev, stdvar, sum}

{%- for item in aggr %}
SELECT {{item}}(c) from t;
{%- endfor %}

This is what it looks like for demo in description.

That is the problem I'm referring to. things like {%- for item in aggr %} is exposed to the query section, making the input query "ugly". I'm targeting a form that's closer to the one in this issue's description.

Another undetermined problem is whether we should render the expanded query in .result file. The content might be overwhelming.

This sounds not a problem for me, if we ignore the rendered query, this will make it hard to debug, so we need to write them to result file.

I'm afraid rendering them out might make the file unreadable. Considering an insert with 10000 values.

@jiacai2050
Copy link
Member

is exposed to the query section, making the input query "ugly". I'm targeting a form that's closer to the one in this issue's description.

Template is complex, sooner or later we will need a DSL, minijinja is a popular one and it has all features we need, I don't think we can design a better one than it.

I'm afraid rendering them out might make the file unreadable.

Maybe we can add an option to configure this.

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

Successfully merging a pull request may close this issue.

3 participants