Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

re_building_group_disposal has only a primary key column #47

Closed
3nids opened this issue Dec 6, 2023 · 12 comments
Closed

re_building_group_disposal has only a primary key column #47

3nids opened this issue Dec 6, 2023 · 12 comments
Assignees

Comments

@3nids
Copy link
Contributor

3nids commented Dec 6, 2023

schemaspy spotted this issue

is this expected?

@sjib
Copy link
Contributor

sjib commented Dec 6, 2023

No, this is a mistake - I will fix it

@sjib
Copy link
Contributor

sjib commented Dec 6, 2023

@3nids Do we need an obj_id in these m:n relation tables from a postgres / QGIS perspective?
re_tables_obj_id_necessary_or_not

I just see that I had intended to not add it anymore, as INTERLIS does not need it as far as I understand.

<DSS_2020_1_LV95.Siedlungsentwaesserung.Gebaeudegruppe_EntsorgungAssoc> <EntsorgungRef REF="ch080qwzNG000001"/> <Gebaeudegruppe_EntsorgungAssocRef REF="ch080qwzGG000001"/> </DSS_2020_1_LV95.Siedlungsentwaesserung.Gebaeudegruppe_EntsorgungAssoc>

and
<!-- <DSS_2020_1_LV95.Siedlungsentwaesserung.Erhaltungsereignis_AbwasserbauwerkAssoc TID="ch080qwzRA007000">--> <DSS_2020_1_LV95.Siedlungsentwaesserung.Erhaltungsereignis_AbwasserbauwerkAssoc> <AbwasserbauwerkRef REF="ch080qwzSW000142"/> <Erhaltungsereignis_AbwasserbauwerkAssocRef REF="ch080qwzEH001005"/> </DSS_2020_1_LV95.Siedlungsentwaesserung.Erhaltungsereignis_AbwasserbauwerkAssoc> <DSS_2020_1_LV95.Siedlungsentwaesserung.Erhaltungsereignis_AbwasserbauwerkAssoc> <AbwasserbauwerkRef REF="ch080qwzKA001110"/> <Erhaltungsereignis_AbwasserbauwerkAssocRef REF="ch080qwzEH001128"/> </DSS_2020_1_LV95.Siedlungsentwaesserung.Erhaltungsereignis_AbwasserbauwerkAssoc>

@ponceta
Copy link
Member

ponceta commented Dec 7, 2023

An id can sometimes be a good thing to have. (automated serial id not NULL would be sufficient)
An obj_id is maybe too much (do we consider the link between two objects as an object?)
On the tecnical part, the combination of both fk_ keys is sufficient as primary key and adding not NULL and reference constraints on both fk fields is a good way to go.

@3nids
Copy link
Contributor Author

3nids commented Dec 7, 2023

In a pure DB vision, a single primary key is useless as we could use compound keys on the 2 reference columns.
But QGIS needs a primary key in a layer to work properly.

And yes, please use a UUID (not necessarily a TWW OID, a simple auto generated UUID is fine) as it is much more robust than serials.

@sjib
Copy link
Contributor

sjib commented Dec 7, 2023

@3nids So how should this be then adapted with an UUID? I suggest to then not name it obj_id but uuid_id

CREATE TABLE tww_od.re_building_group_disposal ( obj_id varchar(16) NOT NULL, CONSTRAINT pkey_tww_od_re_building_group_disposal_obj_id PRIMARY KEY (obj_id) ) WITH ( OIDS = False ); CREATE SEQUENCE tww_od.seq_re_building_group_disposal_oid INCREMENT 1 MINVALUE 0 MAXVALUE 999999 START 0; ALTER TABLE tww_od.re_building_group_disposal ALTER COLUMN obj_id SET DEFAULT tww_sys.generate_oid('tww_od','re_building_group_disposal'); COMMENT ON COLUMN tww_od.re_building_group_disposal.obj_id IS 'INTERLIS STANDARD OID (with Postfix/Präfix), see www.interlis.ch';

@3nids
Copy link
Contributor Author

3nids commented Dec 7, 2023

if not already install we need CREATE EXTENSION IF NOT EXISTS "uuid-ossp";

and then you can use uuid_generate_v1() as default.

for the column, just go for id, simpler is better :)

@ponceta
Copy link
Member

ponceta commented Dec 7, 2023

If there's no preference, I would go with uuid_generate_v4 ()

uuid_generate_v4 () → uuid
Generates a version 4 UUID, which is derived entirely from random numbers.

MAC adress are considered as personnal data and I would rather avoid anything to do with RGPD in that context...

@sjib
Copy link
Contributor

sjib commented Dec 7, 2023

Does this look good - I just created a table uuidtest:
CREATE TABLE IF NOT EXISTS test.uuidtest
(
id uuid NOT NULL DEFAULT uuid_generate_v4(),
CONSTRAINT uuidtest_pkey PRIMARY KEY (id)
)
`
Documentation see https://www.postgresql.org/docs/16/uuid-ossp.html#UUID-OSSP-FUNCTIONS-SECT

@sjib
Copy link
Contributor

sjib commented Dec 7, 2023

20231207_test_uuid_generate

@ponceta @3nids Is this what you are expecting?

@3nids
Copy link
Contributor Author

3nids commented Dec 7, 2023

yes

@cymed
Copy link
Contributor

cymed commented Dec 7, 2023

looks good to me. Associations should not require OIDs

@sjib
Copy link
Contributor

sjib commented Dec 12, 2023

Solved with #49

@sjib sjib closed this as completed Dec 12, 2023
cymed added a commit to cymed/TEKSI-wastewater that referenced this issue Aug 14, 2024
* Move alterations from closed sql_fixes to new PR

The not yet working part is now separated

* do not materialize gepknoten

* gepknoten minor fix

* add MATERIALIZED VIEW {ext_schema}.knoten_bauwerksattribute

* unconnectected_node bwrel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants