-
Notifications
You must be signed in to change notification settings - Fork 2
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
cerbund-contact: schema consistency n-to-m tables #5
Comments
* Repairs command delete. ** It was attempted to delete the wrong role entry, because the wrong 'id' was used, helped by the db inconsistency https://github.com/Intevation/intelmq/issues/19 ** fixes some calls and avoid too early commits * Adds a more detailed logging level DDEBUG. * Adds more logging for database operations, especially to see when commits or rollbacks are done.
That is by design. They don't need an ID, because each tuple is identified by the The
It's good practice to always use e.g. the table name to qualify the column names. Of course, it's also good practice to consistently use the same column name for the same data, e.g. always refer to the organisation's ID as So, yes, I think we should be more consistent. They main points seem to be:
|
We had a discussion about this today. The plan now is:
|
Those tables also have the |
Implemented with revision certtools/intelmq@9f4c6af |
https://github.com/Intevation/intelmq/blob/43c830a747a45464747bed61bdd8d74f50c66a21/intelmq/bots/experts/certbund_contact/initdb.sql
may be inconsistent how column names in n-to-m tables are used.
The following n-to-m tables:
organisation_to_network
organisation_to_asn
organisation_to_fqdn
do not have a
PK
calledid
but use two prefixed names to the linked to tables.Example:
While the n-to-m tables
role
andinhibition
have a primary keyid
.This has the effect:
id
twice.I have a feeling we should possibly make this more consistent at one point
or document the design decisions.
(So far I've have not fully thought through the problem.)
@bernhard-herzog What is your take on it?
The text was updated successfully, but these errors were encountered: