chronomodel
chronomodel copied to clipboard
Better check for whether or not a table is_chrono?
The previous check didn't work properly because data_source_sql
(which data_source_exists?
uses internally) only qualifies the table_name to the current schema if a fully qualified name isn't passed.
This could be made even better if the schema was stripped off table
before it's passed in, as
right now this won't work if you pass a fully qualified name in to change_table
and also pass temporal: true
Ref: https://apidock.com/rails/v5.1.7/ActiveRecord/ConnectionAdapters/PostgreSQL/SchemaStatements/data_source_sql and https://apidock.com/rails/v5.1.7/ActiveRecord/ConnectionAdapters/PostgreSQL/SchemaStatements/quoted_scope
Looks like this project has begun to be maintained again, so I've merged upstream master. If this PR is still useful would it be possible to get it in the next release?
Hi, any chance to rebase instead of merge and provide a failing test case?
@tagliala Yes, I should be able to do that. Most likely won't be until next week, though.
@tagliala This is ready for review :)
Hi, I can suggest to run GitHub actions on your fork to fix specs on older ruby and rails versions
NoMethodError:
undefined method `uuid' for Random:Class
@tagliala I added a spec that did a great job of highlighting where things currently aren't working (having a "." within the table name) and fixed all the issues I could find around it. I tested locally against AR 5/5.2/6/7 and everything appears to be working fine on those AR versions. I'll push a fix for Random.uuid
not being available in older versions of Ruby.
Hi @tagliala following up about this to see where it's at. Thank you :pray: !
Hi @tagliala Following up about this (and #97), thank you!
Any plans on this and #97 get merged? Thank you!
Hi, thanks for your interest in ChronoModel
I can't guarantee that this will be merged. We don't want to introduce new issues in this component and all we are fine with this limitation. Also, I'm concerned about new issues that this can introduce by allowing a different schema path
As per the PR itself, I've left a comment. I'm also interested in a better management of quoted chars in the table name, I would like to understand if it is possible to avoid checks like table_name[0] != '"'
and
def quote_identifier_name(prefix: "", table: "", suffix: "")
if table[0] == '"' && table[-1] == '"'
"\"#{prefix}#{table[1..-2]}#{suffix}\""
else
"#{prefix}#{table}#{suffix}"
end