-
Notifications
You must be signed in to change notification settings - Fork 11
add structure for registering logdata during table imports. #52
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
base: main
Are you sure you want to change the base?
Conversation
JacobusXIII
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great first version! Now it needs some polishing up ;)
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/system-constants.sql
Outdated
Show resolved
Hide resolved
JacobusXIII
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be more specific about naming? Like table_data_metadata instead of just metadata or so?
@BertScholten what do you think?
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/system-constants.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One tiny issue.
And I'm not sure if i like the (universal) term metadata. Can we come up with something better?
It's the metadata of the imported data.....
|
|
decided: metadata table will be renamed to "load_table_logs" |
JacobusXIII
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The action being performed is registering all load_tables, which result in een entry in the load_table_logs table. That's why I'm in favor of the name register_load_table as a function-, boolean- and contants-name.
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load_table_logs-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/system-constants.sql
Outdated
Show resolved
Hide resolved
JacobusXIII
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last few issues
common/src/main/sql/database-build/essentials/system-constants.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-store-data.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/load-table-logs.sql
Outdated
Show resolved
Hide resolved
common/src/main/sql/database-build/essentials/system-constants.sql
Outdated
Show resolved
Hide resolved
BertScholten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit late to the party perhaps (went over open PRs to check if anything could be closed), but had a suggestion.
common/src/main/sql/database-build/essentials/system-constants.sql
Outdated
Show resolved
Hide resolved
…he default value of the should_register_load_table to TRUE. With this change, de build default is to register the metadata in the table log data.
BertScholten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds a log table for logging import-table logdata and functionalily to fill it.
This table is filled during the database-build with some metadata from the imported import-files, like the name of the file, the import-time and the checksums of the table before and after the import. This is done by a altered function of "load_table".
This overwiew can be used for checks on the database build(are the correct import-files used during the build-proces?).
Also, often is the question what import-file is used for a particular table. This information is placed in the .html- and .rtf files, but these stand apart from the database, so if those can't be found, there is no way to know.
With the introduction of this log-table, the used import-files can be easy to find.