Tanagra-Aou-Integration-Updated for AoU changes#412
Conversation
chenchals
force-pushed
the
chenchals/DT-297-Tanagra-AoU-local-dev
branch
from
b95d2af to
7522b95
Compare
marikomedlock
left a comment
marikomedlock
left a comment
There was a problem hiding this comment.
Thanks for this work! It's great to see it working in the tests.
| - column: | ||
| name: selection_data | ||
| type: text | ||
| type: ${text.type} | ||
| constraints: | ||
| nullable: false | ||
| - column: | ||
| name: ui_config | ||
| type: text | ||
| type: ${text.type} |
There was a problem hiding this comment.
These two fields are serialized json and may be longer than 255 characters. What do you think about using longtext for these fields in MySql?
There was a problem hiding this comment.
Several alternatives: varchar(65xxx), longtext can hold up to 4GB data. Done!
There was a problem hiding this comment.
OK either sounds fine: longer varchar (maybe 25k is enough) or longtext. We can always change the datatype in the future if we find we're hitting the limit. I think we'd hit the 255 limit fairly quickly though.
| - column: | ||
| name: key | ||
| type: text | ||
| type: ${_text.type} |
There was a problem hiding this comment.
This column is part of the unique constraint, should it be id.type instead?
There was a problem hiding this comment.
yes, if it is part of unique constraint, varchar(255) is OK too if key length if >50. Done!
| databaseChangeLog: | ||
| - property: | ||
| dbms: postgresql | ||
| name: _id.type |
There was a problem hiding this comment.
Why are the names prefixed with an underscore in this changeset file and not in the previous one? I think it would be easier to follow if they are the same in both places.
There was a problem hiding this comment.
no reason, not sure if property names must be unique across changelogs. I will remove the _ and check. Updated with global properties
| name: key | ||
| type: text | ||
| type: ${_text.type} |
There was a problem hiding this comment.
Same as above. This column is part of the unique constraint, does it need to be id.type?
There was a problem hiding this comment.
yes, if it is part of unique constraint, varchar(255) is OK too if key length if >50. Done!
| name: enum | ||
| type: text | ||
| type: ${_text.type} |
There was a problem hiding this comment.
Same as above. This column is part of the unique constraint, should it be id.type?
There was a problem hiding this comment.
yes, if it is part of unique constraint, varchar(255) is OK too if key length if >50. Done!
2 participants
The changes include:
propertiesforcolumn-typesfor differentdbms