perseas / Pyrseas

Provides utilities for Postgres database schema versioning.
https://perseas.github.io/
BSD 3-Clause "New" or "Revised" License
395 stars 67 forks source link

Add support for parallel safe functions and partial aggregations for 9.6 #161

Closed tbussmann closed 7 years ago

tbussmann commented 7 years ago

With PostgreSQL 9.6 one of the major new features is the support for parallel query. To support this, functions and aggregates have to be qualified to be parallel safe, restricted or unsafe. Furthermore aggregates got the possibility to be calculated in parts by specifying a combine function. It would be great if Pyrseas would support these new qualifiers. Links:

jmafc commented 7 years ago

Design notes: This will require modifying the query in pyrseas/dbobject/function.py to fetch the new proparallel column from catalog pg_proc. We can also add a SAFETY_TYPES dict similar to the VOLATILITY_TYPES for the values that proparallel can take (r: restricted, s: safe or u: unsafe).

For the aggregates, the query will need to retrieve the new columns aggcombinefn, aggserialfn and aggdeserialfn. It looks like we're also missing other columns added in 9.4 to deal with moving aggregates, namely, aggmtransfn, aggminvtransfn, aggmfinalfn, aggmfinalextra, aggmtranstype, aggmtransspace and aggminitval.

jmafc commented 7 years ago

@tbussmann I've started working on this and believe I could use some help writing appropriate tests for all these new attributes and PG version combinations. They don't have to be fancy, but should cover at least once each of the attributes on both the map-to-YAML and generate-SQL sides. The new YAML attributes are all named as per the SQL syntax, e.g., sspace, finalfunc_extra, msfunc, minvfunc, etc. If you have some time to sketch (or write) some tests, it would be greatly appreciated.

tbussmann commented 7 years ago

great! I'd be happy if I can help with test-coverage. But first, I'll have to see how I can use the code in master and how the test suite works in general as I'm not a python guy ;) How are you going to deal with the missing ALTER AGGREGATE support? Issue a DROP and CREATE on mismatch? This can lead to a dependency hell, but I don't have a better idea so far.

jmafc commented 7 years ago

ALTER AGGREGATE support does exist in part due to inheritance from DbObject. Although there's no unit test currently, the SQL only supports RENAME, changing the OWNER or moving it to a new SCHEMA. I believe if I were to add a unit test for the second item, it would work. I realize you're probably talking about changing something like a FINALFUNC from thisfinal to anotherfinal. Those can only be dealt by DROP and CREATE. We have implemented that for constraints and it appears to work well. The difference is that with aggregates we'd have to compare about a dozen separate attributes, instead of a CHECK rule or column list. In any case, altering aggregates probably deserves its own separate issue (I'm not sure we can get it done for 0.8--at least if we want to release it in the near future).

The aggregate test code is in https://github.com/perseas/Pyrseas/blob/c4f2665d0176920725deeb3fb2e5b08c900f9b03/tests/dbobject/test_function.py#L315 but I'm agreeable to splitting it to a separate file if preferred. You don't need to write the Python tests (unless you want to). You can sketch the types of tests that need to be added. Most of the test code is expressed in something very similar to JSON on the ToMap side and JSON with the resulting SQL on the ToSql side.

jmafc commented 7 years ago

@tbussmann Tobias, I'm not trying to pressure you, but I'm about ready to start coding the additional agggregate tests and functionality. I've added (but not submitted) for mapping moving aggregates and tomorrow I'll add the SQL generation test(s) for moving aggregates and maybe start working on the partial aggregates. If you're too busy, I understand. You can always look over what I commit and suggest any improvements when you have time.

tbussmann commented 7 years ago

Thanks, @jmafc, I’m impressed by the activity of development this project is receiving recently. You’re right, unfortunately I wasn’t able to work on the tests over the weekend as I wanted. I spend the rest of the week at pgConf.eu so I won’t be able to support it in the next days either. I will give feedback if I spot any missing cases in what you’re working on once committed. Thanks again for all your work!

jmafc commented 7 years ago

I submitted the moving-aggregate changes but without full test coverage of the various options (e.g., msspace, mfinalfunc). Before dealing with the partial aggregates, however, it seems appropriate to add support for ordered-set and hypothetical-set aggregates (also added in 9.4), but it seems I may first want to revisit some of the regular function support (variadic, window, etc.).