Go to string_agg (again)

Bug #1441750 reported by Ben Shum on 2015-04-08
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned
3.1
Undecided
Unassigned
3.2
Undecided
Unassigned

Bug Description

Evergreen master

So, it seems that we have some older style ARRAY_TO_STRING(ARRAY_AGG()) (and similar) back in the base schemas and code. Let's change those over to native STRING_AGG() functions (present in PG 9+) if we can.

See old bug 874296 for original report regarding these kind of changes.

Try, try again. Working branch forthcoming.

Ben Shum (bshum) wrote :

Looks like we got it present in

030.schema.metabib.sql -- function metabib.reingest_record_attributes
990.schema.unapi.sql -- function unapi.mmr

Searching for other cases and will actually build a working branch to fix this, this time.

tags: added: database performance
Changed in evergreen:
status: New → Triaged
status: Triaged → In Progress
assignee: nobody → Ben Shum (bshum)
importance: Undecided → Medium
Ben Shum (bshum) wrote :

Noting there is also an array_accum() used in t/search_limit_facet_fetch.pg that should be changed with array_agg()

Dan Scott (denials) wrote :

We should probably also add a test for these and any other known problematic functions to prevent them from getting copy-pasted back into existence in the future...

Ben Shum (bshum) wrote :

Working branch to address the ones I've been able to find -- user/bshum/lp1441750

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/bshum/lp1441750

Tested with a clean DB, adding the upgrade scripts, and also using a refreshly rebuilt concerto, and search seems to work fine.

Changed in evergreen:
milestone: none → 3.2-beta
status: In Progress → Confirmed
assignee: Ben Shum (bshum) → nobody
tags: added: pullrequest
Jason Stephenson (jstephenson) wrote :

I added a signoff branch here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1441750-signoff

working/user/dyrcona/lp1441750-signoff

I tested it and don't see any problems in search, but I think more eyes would be helpful. It might also be useful for release notes if someone could generate some performance metrics from before and after this patch.

tags: added: signedoff
Changed in evergreen:
milestone: 3.2-beta → 3.2-rc
Changed in evergreen:
milestone: 3.2-rc → 3.2.1
Changed in evergreen:
milestone: 3.2.1 → 3.2.2
Ben Shum (bshum) wrote :

Hmm, looks like we added a new array_to_string(array_agg()) in 999.functions.global.sql with the actor.usr_merge function with new components for 3.2. We'll need to change that as well moving forward.

Changed in evergreen:
milestone: 3.2.2 → 3.2.3
Changed in evergreen:
milestone: 3.2.3 → 3.2.4
Changed in evergreen:
milestone: 3.2.4 → 3.2.5
Changed in evergreen:
milestone: 3.2.5 → 3.3-rc
assignee: nobody → Jason Stephenson (jstephenson)
Jason Stephenson (jstephenson) wrote :

I'm setting this to won't fix for 3.1 because I'm updating the upgrade script for name keywords in acrtor.usr_merge, and that feature does not exist in 3.1.

Jason Stephenson (jstephenson) wrote :

I pushed a rebased, updated branch to working/user/dyrcona/lp1441750-latest. This branch has an additional commit on top to update the actor.usr_merge function as mentioned by Ben in comment #6. I am also removing the signedoff tag, even though all pgtap and Perl tests pass.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1441750-latest

tags: removed: signedoff
Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
Changed in evergreen:
milestone: 3.3-rc → 3.3.1
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers