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

Add support for combination queries on same snapshot point #439

Open
wants to merge 1 commit into
base: mas-d31-i433re2capture
Choose a base branch
from

Conversation

martinsumner
Copy link
Owner

Allow for return_keys of logical combinations of queries

Allow for return_keys of logical combinations of queries

OR : {token, {'UNION', TokenLine}}.
AND : {token, {'INTERSECT', TokenLine}}.
NOT : {token, {'SUBTRACT', TokenLine}}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Kind, of, NOT is in fact a unary operation and SUBTRACT a binary.

This might cause future confusion. If you write NOT $d, then you mean to say the universe apart from whatever is in $d. But the universe is not part of your language.

  • But more prominently, if I write $a SUBTRACT NOT $b you fail, whereas in set theory you would take the complenet from B and subtract that from A.

It will be hard to implement complement. So not sure you want to add NOT. Also because NOT is very hard to do in regular expressions.

Copy link
Contributor

Choose a reason for hiding this comment

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

In many applications this parser would have a "normalize" function that applies "De Morgan" laws to normalize the SET. This may simplify the final regular expression building, but may be considered out of scope here.

INTERSECT : {token, {'INTERSECT', TokenLine}}.
SUBTRACT : {token, {'SUBTRACT', TokenLine}}.

\$[1-8]+ : {token, {set_id, TokenLine, strip_identifier(TokenChars)}}.
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels limiting... zero and nine cannot be in the set_id? Do you actually mean a number not starting with zero?
\$[1-9][0-9]*

Copy link
Contributor

Choose a reason for hiding this comment

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

In particular since you take the "nth" set_id from a list, missing the "9" is confusing if you allow "11".
I like the map you use in filters better, then it does not really matter what representation you use. Is that possible here as well, to provide apply_setop with a map of set_id to the actual set?


apply_setop({setop, SetOp}, SetList) ->
apply_setop(SetOp, SetList);
apply_setop(
Copy link
Contributor

Choose a reason for hiding this comment

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

base case missing:

apply_setop({set_id, _, SetID}, SetList) ->
      lists:nth(SetID, SetList)

This follows from

9> F =  leveled_setop:generate_setop_function("$1").
#Fun<leveled_setop.0.97347034>
10> F([[]]).
** exception error: no function clause matching leveled_setop:set_function(set_id) (/Users/thomas/Quviq/Customers/NHS/leveled/src/leveled_setop.erl, line 54)
    in function  leveled_setop:apply_setop/2 (/Users/thomas/Quviq/Customers/NHS/leveled/src/leveled_setop.erl, line 49)

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