Page MenuHomeSoftware Heritage

[WIP] Add a composite swhid type in postgresql
AbandonedPublicDraft

Authored by douardda on Feb 1 2021, 5:05 PM.

Details

Summary

Note: this is a support for discussion, not a diff to review.

Depends on D4985.

Implement support for SWHID on Origin

XX

Diff Detail

Event Timeline

Build has FAILED

Patch application report for D4986 (id=17785)

Rebasing onto f6ae8a062d...

Current branch diff-target is up to date.
Changes applied before test
commit dc470d73d20ea56d57298fe3833e54858e5876bc
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 15:49:32 2021 +0100

    Implement support for SWHID on Origin
    
    XX

commit 428efe451219839cf9c646918f469cac373d093d
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 10:56:02 2021 +0100

    Add a composite swhid type in postgresql

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1116/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1116/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 1 2021, 5:05 PM
Harbormaster failed remote builds in B18926: Diff 17785!
olasd added inline comments.
swh/storage/postgresql/converters.py
147

Optional[SWHID]

swh/storage/postgresql/db.py
34

Eeeeek.

If we go for a regexp approach, this should be a re.fullmatch. But of course it'd be much, much better to avoid using a regexp altogether.

56

spurious debug print

87–110

When we used to do this kind of query for bytea and bytea[]s (well, we really only did a select NULL::bytea, NULL::bytea[]), we spent a substantial amount of time querying for the oid of these types instead of doing actual database queries.

We really need to make sure this only happens once per connection instance, even in pooled situations. Unfortunately, unlike for byteas, we won't be able to hardcode the oid.

1243–1244

Remembering to add these parentheses is going to be annoying. are they really necessary?

swh/storage/postgresql/storage.py
1248–1263

I think I prefer the previous, better-typed form of this call, even if it's verbose and (silently) breaks when we change the RawExtrinsicMetadata fields.

swh/storage/sql/30-schema.sql
39–40

for any pronounciation that I can think of, this should probably be "a SWHID" :-)

douardda added inline comments.
swh/storage/postgresql/db.py
34
87–110

ok something to keep in mind...

1243–1244

I thought they are, reading:

https://www.postgresql.org/docs/12/rowtypes.html#ROWTYPES-ACCESSING

Actually for comparing the whole composite object it might not be required, This seems to work fine:

gres=# select * from on_hand where on_hand.item=ROW('fuzzy dice', 42, 1.99)::inventory_item;
          item          | count 
------------------------+-------
 ("fuzzy dice",42,1.99) |  1000

I'll give a try without.

olasd's comments + more cleanups

Build has FAILED

Patch application report for D4986 (id=17930)

Rebasing onto efd8815b89...

First, rewinding head to replay your work on top of it...
Applying: Add a composite swhid type in postgresql
Applying: Implement support for SWHID on Origin
Changes applied before test
commit f4f65a183be64837404eeb9563998afda12edb8a
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 15:49:32 2021 +0100

    Implement support for SWHID on Origin
    
    Cassandra is still WIP

commit 41802de66908aa797dc29e1b942f3b42ffeab154
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 10:56:02 2021 +0100

    Add a composite swhid type in postgresql

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1138/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1138/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 5 2021, 3:10 PM
Harbormaster failed remote builds in B19049: Diff 17930!

Build has FAILED

Patch application report for D4986 (id=17931)

Rebasing onto efd8815b89...

First, rewinding head to replay your work on top of it...
Applying: Add a composite swhid type in postgresql
Applying: Implement support for SWHID on Origin
Changes applied before test
commit 1d99f20730492bc55d3106ee78d48329e4555c54
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 15:49:32 2021 +0100

    Implement support for SWHID on Origin
    
    Cassandra is still WIP

commit cbf71255ebae3bab442cac7ed0bedbfef9d24726
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 10:56:02 2021 +0100

    Add a composite swhid type in postgresql

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1139/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1139/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 5 2021, 3:14 PM
Harbormaster failed remote builds in B19050: Diff 17931!

Build has FAILED

Patch application report for D4986 (id=17932)

Rebasing onto efd8815b89...

Current branch diff-target is up to date.
Changes applied before test
commit 3a0617c8eb1868986d1c45df6954db0f7978f8e4
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 15:49:32 2021 +0100

    Implement support for SWHID on Origin
    
    Cassandra is still WIP

commit 4ebd18eaf8cd7f5f16a5e3c85b37ea0e09336b38
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 26 10:56:02 2021 +0100

    Add a composite swhid type in postgresql

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1140/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1140/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 5 2021, 3:16 PM
Harbormaster failed remote builds in B19051: Diff 17932!