Skip to content

Review #2304 for code improvements: guards for a missing .query in set operations #2646

Open

Description

@mathiasrw

This is not a bug report. It is an invitation to look at #2304 again, because it may guard against a symptom rather than fix the cause. The goal is to understand the change and decide whether anything should be improved. The notes below come from a quick read of the diff, not from a full analysis.

The concern:

  • Guards instead of a cause. In queryfn3 (src/38query.js) eight places now check query.unionallfn.query && ... (and the same for unionfn, exceptfn, intersectfn) before setting the right operand's modifier. The question is why the compiled function of a set operation sometimes has no .query. If that case is legitimate, setting the modifier where the operand is compiled, in one place, would remove the eight checks.
  • Evaluate UNION / EXCEPT / INTERSECT chains left to right to close #202 #2582 (set operation chains, 5.0) replaces these branches with one applySetOp; the guards may be worth folding into that.

Related: #635, #2582.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions