age icon indicating copy to clipboard operation
age copied to clipboard

Oid investigation

Open muhammadshoaib opened this issue 3 years ago • 3 comments

This branch tests uses of namespaces as OIDs instead of generating new one. We can solve this issue of OID using this way without adding much complex solution.

muhammadshoaib avatar Mar 18 '22 13:03 muhammadshoaib

I pulled it down and the build failed -

CREATE EXTENSION
============== running regression test queries        ==============
test graphid                      ... ok
test agtype                       ... ok
test cypher                       ... ok
test catalog                      ... FAILED
test scan                         ... ok
test expr                         ... FAILED
test cypher_create                ... ok
test cypher_match                 ... ok
test cypher_unwind                ... ok
test cypher_set                   ... ok
test cypher_remove                ... ok
test cypher_delete                ... ok
test cypher_with                  ... ok
test cypher_vle                   ... ok
test cypher_union                 ... ok
test cypher_merge                 ... ok
test age_load                     ... ok
test drop                         ... ok
============== shutting down postmaster               ==============

=======================
 2 of 18 tests failed.
=======================

Additionally, you need to make your commit comments more verbose -

commit 2fc8d73f284a8d7f827c05643c7953fa83d9281c
Author: Shoaib <[email protected]>
Date:   Wed Mar 9 10:04:38 2022 +0100

    modified label ids

commit e99c2c785e83389fd6e8f4266df911117919ed17
Author: Muhammad Shoaib <[email protected]>
Date:   Wed Mar 9 08:30:23 2022 +0100

    modified graph id from integer to serial

commit 4ef46d0aaec741e541d44ba122e0e80602b5d67a
Author: Muhammad Shoaib <[email protected]>
Date:   Wed Mar 2 00:18:05 2022 +0100

    inital changes in graphid

jrgemignani avatar Apr 11 '22 20:04 jrgemignani

Both tests are failing because I didn't update the regression tests without consultations. Please have a look at diff file and advise if it is safe to update regression tests.

muhammadshoaib avatar Apr 11 '22 21:04 muhammadshoaib

I'm not so sure about using graphid, I do believe this was mentioned before with Alex's changes. graphid is too similar, if not identical, to other uses that it will cause confusion. If we are using the namespace as the id, we should name it so that is obvious. So, something like, graph_namespace_id. Additionally, we should have some comments stating this change - I might have missed them.

jrgemignani avatar Apr 11 '22 21:04 jrgemignani

@muhammadshoaib Is this PR still needed?

jrgemignani avatar Jan 05 '23 01:01 jrgemignani

This pull request have already been incorporated in other pull requests for PG12 and PG13

muhammadshoaib avatar Feb 17 '23 15:02 muhammadshoaib