Code review comment for lp:~wgrant/storm/distinct-on

Revision history for this message
Thomas Herve (therve) wrote :

It's relatively unfortunate to change the global compiler, as it's only supported on Postgres. OTOH it should fail pretty obviously on other databases, so I guess it's fine.

[1] It would be nice to update the docstring of Store.config to mention the new format for distinct (if you're using Postgres).

[2] The state.push calls should be moved before the compilation of the columns.

[3] Please add a note in the NEWS file mentioning the feature.

Thanks, merge once fixed!

review: Approve

« Back to merge proposal