Support SELECT * EXCEPT (col1, col2)-style functionality

[KingMob]

This adds SELECT * EXCEPT (col-i-dont-want, col-that-annoys-me) ... functionality to GenSQL.

Adding * EXCEPT to other areas (conditioning, generation) will come in future PRs.

[sritchie]

interesting, do you recommend leaving this in for production code? I like the idea of more logging via tap>, I just don't know good practices yet.

[KingMob]

Zane requested this. The nice thing about tap> is it's very lightweight if no taps are added.

The less nice thing about tap> is well, everything else, IMO. :P I've tried REBL and Portal, and have yet to find a tap/datafy visualizer I actually think is worth using over logs. They're just so slow to navigate in.

For now, I've taken to running:

(do
  (when (ns-resolve 'user 'log-tap)
    (remove-tap user/log-tap))
  (intern 'user 'log-tap #(do (prn %) (println)))
  (add-tap user/log-tap))

so I get some semblance of logs.

[zane]

I’m happy to be overruled here if you two align on a different solution for logging!

[zane]

@sritchie What, if anything, is OpenGen using for logging?

[sritchie]

Nothing now... in Emmy I have used Timbre but I'm not doing anything special to consume the logs.

[KingMob]

I guess the question is, what problem are we trying to solve? Is this for us, in our debugging efforts? Is this for debugging some eventually-deployed cloud system? Is this for 3rd-party users to get more insight into debugging their own queries? Some weighted combination of all of the above? We can't choose a solution when we haven't articulated the problem.

FWIW, I'm a fan of mulog, and other structured logging libs like pedestal.log. Timbre's not structured, iiuc, but it's already cross-platform, and it eliminates much of the need to fiddle with Java logging backend config files, which are both major pluses, especially for downstream users.

For now, I think it's fine to leave the tap>'s in, but I might add a logging helper for it.

[KingMob]

We can't choose a solution when we haven't articulated the problem.

For me, I find it help for debugging, since I can see all output at once, and search the REPL if necessary. Cloud servers are a future problem, imo. And I feel I don't know enough about third-party users yet to offer an opinion.

[sritchie]

can this ever fail? Can we get an empty selections input?

[KingMob]

Good question. I checked, but all my attempts result in parse errors. I don't think we can reach this code without something in the selections.

[sritchie]

do you want to handle this, or make a ticket to track this?

[KingMob]

Yeah, the next step wasn't very clear. I thought Zane might be the person to ask, but he's not around. I suspect Ulli won't care much.

I dug around a bit to see which dbs do this, and the behavior I'm thinking of is an old, nonstandard default of MySQL's (what I cut my teeth on). In that light, it's probably better to delete this.

[KingMob]

Removed.

[sritchie]

let's get this one resolved. Reviewing this sort of PR is where I miss types, I can't see what attrs should be...

[KingMob]

I'm 99% certain it's a vec of keys (which should be strings these days), based on the attribute code I see elsewhere. In that case, it should almost certainly be clojure.core/name here.

I think the reason nobody's noticed the error is because the select-attrs fn is only called in i.q.relation/project, which is never used.

[KingMob]

Yep, that's it:

(-> (strict/q "SELECT * FROM tbl LIMIT 1" db)
    first
    (tuple/select-attrs #{"Launch_Site" "Contractor"}))
=> {}

(defn select-attrs
  "Retrives tuple `tup` with only the attributes in `attrs`."
  [tup attrs]
  (let [names (into #{} (map clojure.core/name) attrs)]               
    (medley/filter-keys (comp names clojure.core/name)
                        tup)))
=> #'inferenceql.query.tuple/select-attrs

(-> (strict/q "SELECT * FROM tbl LIMIT 1" db)
    first
    (tuple/select-attrs #{"Launch_Site" "Contractor"}))
=> {"Launch_Site" "Cape Canaveral", "Contractor" "Lockheed Martin"}

[KingMob]

Fixed.

[KingMob]

@sritchie Got time for a re-review? I think I addressed everything.

[sritchie]

Great work, !

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 75.87%. Comparing base (931aae4) to head (3823297).

Files	Patch %	Lines
src/inferenceql/query/plan.cljc	38.09%	3 Missing and 10 partials ⚠️
src/inferenceql/query/tuple.cljc	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #90      +/-   ##
==========================================
- Coverage   76.26%   75.87%   -0.39%     
==========================================
  Files          30       30              
  Lines        1462     1484      +22     
  Branches       52       60       +8     
==========================================
+ Hits         1115     1126      +11     
- Misses        295      298       +3     
- Partials       52       60       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.