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

cerbund-contact: schema consistency n-to-m tables #5

Closed
bernhardreiter opened this issue Feb 24, 2017 · 4 comments
Closed

cerbund-contact: schema consistency n-to-m tables #5

bernhardreiter opened this issue Feb 24, 2017 · 4 comments

Comments

@bernhardreiter
Copy link
Member

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 called id but use two prefixed names to the linked to tables.
Example:

CREATE TABLE organisation_to_asn (
    organisation_id INTEGER,
    asn_id BIGINT,
    notification_interval INTEGER NOT NULL, -- interval in seconds

    PRIMARY KEY (organisation_id, asn_id),

While the n-to-m tables role and inhibition have a primary key id.
This has the effect:

  1. when joining role and contact, we get id twice.
  2. While there is only one organisation_to_asn entry possible for a certain combination of organisation and id, we can have several role entries for a certain combination of organisation and contact.

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?

bernhardreiter referenced this issue in Intevation/intelmq-mailgen Feb 24, 2017
 * 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.
@bernhard-herzog
Copy link
Member

The following n-to-m tables:

  • organisation_to_network
  • organisation_to_asn
  • organisation_to_fqdn

do not have a PK called id but use two prefixed names to the linked to tables.

That is by design. They don't need an ID, because each tuple is identified by the organisation_id and the other ID because it's not useful to say the e.g. an FQDN is associated with a single organisaion in two different ways. In dev-rework-notification branch these tables don't even have the notification_interval column anymore, so each tuple actually is just the pair of IDs.

The inhibition table will be removed when switching to the dev-rework-notification branch, so we can ignore it for this discussion. The role table allows a single contact to be used as multiple roles in an organisation and therefore cannot simply use the combination of organistion_id and contact_id as primary key. I'm not sure we need that feature, though.

when joining role and contact, we get id twice.

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 organsiation_id, even in the organisation table, which would alleviate the problem as well. However, it's also common practice to use the identifier id for the ID, and that is what the contact DB currently uses.

So, yes, I think we should be more consistent. They main points seem to be:

  • Can a contact have two different roles in a single organisation? If not, remove the ID column.

  • Should the ID column be named id or prefixed with the table name?

@bernhard-herzog
Copy link
Member

We had a discussion about this today. The plan now is:

  • Remove the role tables altogether. Just have an organisation_id column in the contact tables.
    We don't need the role_type and is_primary_contact attributes and we also don't need to
    be able to associate one contact with multiple organisations.

  • Use prefixes for the ID columns consistently. E.g. use organisation_id even for the primary key
    column of the organisation table.

  • Remove the autonomous system tables. They don't contain any information that cannot be derived
    from the organisation_to_asn tables. The comment attribute is not needed. The annotation for
    the manually managed ASNs can be associated directly with the ASN as well (as is effectively the
    case now anyway because number is the primary key in the autonomous system tables)

@bernhard-herzog
Copy link
Member

  • Remove the autonomous system tables.

Those tables also have the ripe_aut_num column, but its value is just the ASN with a prefix of "AS", so that doesn't contain any interesting information either, except perhaps the information that RIPE has information about it.

@bernhard-herzog
Copy link
Member

bernhard-herzog commented Mar 28, 2017

Implemented with revision certtools/intelmq@9f4c6af

@bernhardreiter bernhardreiter transferred this issue from Intevation/intelmq Sep 12, 2019
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

3 participants