Custom explode_array() function significantly slower than native unnest()

[Dan Scott]

* Evergreen 2.0.6
* OpenSRF 2.0.0
* PostgreSQL 9.0

After upgrading to 2.0.6 from 1.6.1.8, searches with "Limit to available" turned on almost inevitably timed out.

Running search.query_parser_fts() with the param_statuses-checking section ripped out returned the query to a reasonable speed, as one would expect. Focusing on that section, I replaced the use of search.explode_array() with unnest() and found that broad searches went from 300 seconds down to half a second or less.

The difference seems outrageous, particularly as running individual tests of search.explode_array() vs. unnest() showed a negligible difference that should not, in theory, add up to such an astounding amount. So something else is/was probably going on with our database. But - going with native functions over custom SQL functions is the right direction in any case.

[Dan Scott (denials)]

Pushed a cleanup of array_accum() -> array_agg() and *.explode_array() -> unnext() in branch user/dbs/native-array-functions

[Ben Shum (bshum)]

We're also using Evergreen 2.0.6, OpenSRF 2.0.0, but with PostgreSQL 8.4.

Tested replacing with unnest() in the search function for status and location. Subsequently, we saw advanced search using "limit to available" go from 13 seconds to 1 second (for a single record result) and limiting a single copy location actually returned results instead of timing out. These are very significant improvements!

Will test the other parts of the new branch as soon as possible.

[Mike Rylander (mrylander)]

Dan,

I suspect that the difference is caused by the default row count estimates for explode_array() -- 1000, unless adjusted -- where as (I bet, but don't know) Pg knows exactly how many rows will come out of unnest(). That would obviously be important for plan cost decisions, especially when the estimate is off by orders of magnitude.

Anyway, that's my theory. But theories aside, I'm 100% for pushing this ASAP.

-miker

[Dan Scott (denials)]

Published a new set of branches that focus only on explode_array() -> unnest():

 * master: user/dbs/explode_array_to_unnest
 * rel_2_1: user/dbs/explode_array_to_unnest_2_1
 * rel_2_0: user/dbs/explode_array_to_unnest_2_0

These branches differ from user/dbs/native-array-functions and user/tsbere/native-array-functions by splitting out the database schema upgrade scripts into one script per changed function, as well as not touching array_accum() for now (due to very slight behavioral differences observed between array_accum() and array_agg() that probably do not amount to anything in practice but which would be best avoided when applying to a stable release without extensive testing, IMO).

[James Fournie (jfournie)]

Hi Dan, I'm looking into testing this at Sitka

Just a note of a minor problem -- all of the individual version files add 0550 (rather that 0551, 0552, etc) to the upgrade log.

[Dan Scott (denials)]

Thanks James - I've updated the 2_0 and 2_1 branches to correct that problem. i have also updated the upgrade numbers to go from 0551 through 0556 instead of 0549 through 0554 due to upgrades that have been committed since this branch was created.

[Dan Scott (denials)]

And I've pushed the branches to avoid further numbering conflicts.

[James Fournie (jfournie)]

The 2.0 branch references biblio.peer_bib_copy_map which does not exist in 2.0

[Dan Scott (denials)]

James: I don't think you're using a current version of the rel_2_0 branch. See commit 52db7cd3c9fb40dcba787408c929c2c3b5049e04.