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

Feature CORE-6482 - System table with keywords. #310

Merged
merged 3 commits into from
May 17, 2021

Conversation

asfernandes
Copy link
Member

No description provided.

// Relation 54 (RDB$KEYWORDS)
RELATION(nam_keywords, rel_keywords, ODS_13_0, rel_virtual)
FIELD(f_keyword_name, nam_keyword_name, fld_keyword_name, 0, ODS_13_0)
FIELD(f_keyword_reserved, nam_keyword_reserved, fld_keyword_reserved, 0, ODS_13_0)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the column names: RDB$KEYWORD_NAME, RDB$KEYWORD_RESERVED.

As usual, we are repetitive and maybe not well spelled in case of "KEYWORD RESERVED". I did like RDB$CONFIG columns where every column is prefixed with RDB$CONFIG_

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any strong preference about that, as we always had RDB$RELATION_NAME inside RDB$RELATIONS and so on ;-) With just two fields in RDB$KEYWORDS I don't see it as a problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear if you mean you prefer the prefix maintained in both columns or something different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for confusion, I'm OK with prefix in both columns.

@AppVeyorBot
Copy link

@asfernandes asfernandes linked an issue May 6, 2021 that may be closed by this pull request
// Relation 54 (RDB$KEYWORDS)
RELATION(nam_keywords, rel_keywords, ODS_13_0, rel_virtual)
FIELD(f_keyword_name, nam_keyword_name, fld_keyword_name, 0, ODS_13_0)
FIELD(f_keyword_reserved, nam_keyword_reserved, fld_keyword_reserved, 0, ODS_13_0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any strong preference about that, as we always had RDB$RELATION_NAME inside RDB$RELATIONS and so on ;-) With just two fields in RDB$KEYWORDS I don't see it as a problem.


//// FIXME: ODS version
// Relation 54 (RDB$KEYWORDS)
RELATION(nam_keywords, rel_keywords, ODS_13_0, rel_virtual)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ODS number should be fixed.

@asfernandes asfernandes requested a review from dyemanov May 17, 2021 13:09
@asfernandes asfernandes self-assigned this May 17, 2021
@asfernandes asfernandes linked an issue May 17, 2021 that may be closed by this pull request
Copy link
Member

@dyemanov dyemanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR can be merged. Maybe the "squash and merge" option should be preferred to avoid unnecessary garbage in the commit log.

@asfernandes asfernandes merged commit 3ccba19 into master May 17, 2021
@asfernandes asfernandes deleted the work/rdb-keywords branch May 17, 2021 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System table with keywords [CORE6482]
4 participants