Skip to content

Introduce shortcut syntax for optional bool in WHERE expr#142

Merged
jongleb merged 9 commits intoygrek:ahrefsfrom
jongleb:introduce-syntax
Jan 9, 2025
Merged

Introduce shortcut syntax for optional bool in WHERE expr#142
jongleb merged 9 commits intoygrek:ahrefsfrom
jongleb:introduce-syntax

Conversation

@jongleb
Copy link
Copy Markdown
Collaborator

@jongleb jongleb commented Nov 4, 2024

This PR adds new syntax:

WHERE A? AND B? OR C? which is a shortcut to WHERE @choice { None { TRUE } | Some (params) { A } } AND etc

And considers join_cond as not null by the default

Example:

DDL:

CREATE TABLE test19 (
  a INT NOT NULL,
  b INT NOT NULL
);
CREATE TABLE test20 (
  c INT NOT NULL,
  d INT NOT NULL,
  r TEXT NOT NULL
);

SQL:

SELECT *
FROM test19
LEFT JOIN test20 on test20.c = @test20a
WHERE { c = @choice2 }? 
GROUP BY b;

will be generated the following code:

let invoke_callback stmt =
      callback
        ~a:(T.get_column_Int stmt 0)
        ~b:(T.get_column_Int stmt 1)
        ~c:(T.get_column_Int_nullable stmt 2)
        ~d:(T.get_column_Int_nullable stmt 3)
        ~r:(T.get_column_Text_nullable stmt 4)
    in
    let set_params stmt =
      let p = T.start_params stmt (1 + (match choice2 with Some _ -> 1 | None -> 0)) in
      T.set_param_Int p test20a;
      begin match choice2 with
      | None -> ()
      | Some (choice2) ->
        T.set_param_Int p choice2;
      end;
      T.finish_params p
    in
    T.select db ("SELECT *\n\
FROM test19\n\
LEFT JOIN test20 on test20.c = ?\n\
WHERE " ^ (match choice2 with Some _ -> " c = " ^ "?" ^ " " | None -> " TRUE ") ^ " \n\
GROUP BY b") set_params invoke_callback

Also, it differs from the regular Choice in that, in this case, not a polymorphic variant is generated because for this case, it would lead to the useless matching from the regular option type to the poly version of the option in the user code.

@jongleb jongleb changed the title Introduce optional bool syntax Introduce shortcut syntax for optional bool in WHERE expr Nov 4, 2024
@jongleb jongleb requested a review from ygrek November 4, 2024 17:04
@jongleb jongleb marked this pull request as ready for review November 4, 2024 17:04
@ygrek
Copy link
Copy Markdown
Owner

ygrek commented Nov 27, 2024

didn't look at the code

  1. why is :: Type? here?
  2. So it means if A? OR B? and both are None - then condition is true and everything is selected.
  3. What is expected usage ratio for A! and A? One obviously can be expressed with the other (or via current manual unwrapping), so if one is very low use - i would prefer to not introduce extra syntax.

@jongleb
Copy link
Copy Markdown
Collaborator Author

jongleb commented Dec 2, 2024

why is :: Type? here?

Just shortcut
Remove?

So it means if A? OR B? and both are None - then condition is true and everything is selected.

Yes, something like with COALESCE (maybe a bad analogy), while the first is not null - type is always null

What is expected usage ratio for A! and A? One obviously can be expressed with the other (or via current manual unwrapping), so if one is very low use - i would prefer to not introduce extra syntax.

Ok I'll remove it

@ygrek
Copy link
Copy Markdown
Owner

ygrek commented Dec 3, 2024

ok, so keep this PR only about introducing automatic elimination for A? OR B?
I am also slightly uneasy about ::Type? precisely because it means different thing from WHERE A? which may be possibly confusing? Maybe it is worth to be more explicit with :: Maybe Type or some such or maybe I am overthinking

@jongleb jongleb force-pushed the introduce-syntax branch 4 times, most recently from 0b27da2 to e55b86e Compare January 9, 2025 08:46
@jongleb jongleb merged commit b96f2e5 into ygrek:ahrefs Jan 9, 2025
@jongleb
Copy link
Copy Markdown
Collaborator Author

jongleb commented Jan 9, 2025

@ygrek I removed the unnecessary stuff, made a couple of edits and allowed myself to merge, since this PR is already a month old, if there will any new comments, I will fix them in a separate PR

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.

2 participants