issue_comments

Table actions

26 rows where issue = 621989740

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: created_at (date), updated_at (date)

id ▼ html_url issue_url node_id user created_at updated_at author_association body reactions issue performed_via_github_app
636322089 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-636322089 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDYzNjMyMjA4OQ== simonw 9599 2020-05-30T12:08:43Z 2020-05-30T12:08:43Z OWNER Idea: use a chained API to define a complex transition and then execute it all at once. For example: ```python db["mytable"].transform().rename("col1", "col_1") \ .change_type("col1", float) \ .execute() ``` {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655290625 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655290625 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTI5MDYyNQ== simonw 9599 2020-07-08T05:15:45Z 2020-07-08T05:15:45Z OWNER Ideally this would all happen in a single transaction, such that other processes talking to the database would not see any inconsistent state while the table copy was taking place. Need to confirm that this is possible. Also refs transactions thoughts in #121. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655677099 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655677099 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTY3NzA5OQ== simonw 9599 2020-07-08T18:15:02Z 2020-07-08T18:15:02Z OWNER I'm not so keen on that chained API - it's pretty complicated. Here's an idea for a much simpler interface. Essentially it lets you say "take table X and migrate its contents to a new table with this structure - then atomically rename the tables to switch them": ```python db["mytable"].migrate_table({"id": int, "name": str"}, pk="id") ``` The `migrate_table()` method would take the same exact signature as the `table.create()` method: https://github.com/simonw/sqlite-utils/blob/a236a6bc771a5a6a9d7e814f1986d461afc422d2/sqlite_utils/db.py#L615-L625 {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655677396 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655677396 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTY3NzM5Ng== simonw 9599 2020-07-08T18:15:39Z 2020-07-08T18:15:39Z OWNER Alternative possible names: - `.transform_table()` - `.migrate()` - `.transform()` I'm torn between `.migrate_table()` and `.transform_table()`. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655677909 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655677909 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTY3NzkwOQ== simonw 9599 2020-07-08T18:16:39Z 2020-07-08T18:16:39Z OWNER Since neither the term "transform" or "migrate" are used in the codebase at the moment, I think I'll go with `.transform_table()` - that leaves the term "migrate" available for any future database migrations system (similar to Django's). {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655778058 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655778058 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTc3ODA1OA== simonw 9599 2020-07-08T21:54:30Z 2020-07-08T21:54:30Z OWNER Don't forget this step: > If foreign key constraints are enabled, disable them using PRAGMA foreign_keys=OFF. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655782477 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655782477 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTc4MjQ3Nw== simonw 9599 2020-07-08T22:06:23Z 2020-07-08T22:06:23Z OWNER Thinking about the method signature: ```python def transform_table( self, columns, pk=None, foreign_keys=None, column_order=None, not_null=None, defaults=None, hash_id=None, extracts=None, ): ``` This requires the caller to provide the exact set of columns for the new table. It would be useful if this was optional - if you could omit the columns and have it automatically use the previous columns. This would let you change things like the primary key or the column order using the other arguments. Even better: allow column renaming using an optional `rename={...}` argument: ```python db["dogs"].transform_table(rename={"name": "dog_name"}) ``` {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655783875 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655783875 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTc4Mzg3NQ== simonw 9599 2020-07-08T22:09:51Z 2020-07-08T22:10:16Z OWNER I can have a convenient `change_type={...}` parameter for changing column types too. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655785396 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655785396 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTc4NTM5Ng== simonw 9599 2020-07-08T22:14:10Z 2020-07-08T22:14:10Z OWNER Work in progress: not quite right yet, I need smarter logic for how renamed columns are reflected in the generated `INSERT INTO ... SELECT ...` query: ```python def transform_table( self, columns=None, rename=None, change_type=None, pk=None, foreign_keys=None, column_order=None, not_null=None, defaults=None, hash_id=None, extracts=None, ): assert self.exists(), "Cannot transform a table that doesn't exist yet" columns = columns or self.columns_dict if rename is not None or change_type is not None: columns = {rename.get(key, key): change_type.get(key, value) for key, value in columns.items()} new_table_name = "{}_new_{}".format(self.name, os.urandom(6).hex()) previous_columns = set(self.columns_dict.keys()) with self.db.conn: columns = {name: value for (name, value) in columns.items()} new_table = self.db.create_table( new_table_name, columns, pk=pk, foreign_keys=foreign_keys, column_order=column_order, not_null=not_null, defaults=defaults, hash_id=hash_id, extracts=extracts, ) # Copy across data - but only for columns that exist in both new_columns = set(columns.keys()) columns_to_copy = new_columns.intersection(previous_columns) copy_sql = "INSERT INTO [{new_table}] ({new_cols}) SELECT {old_cols} FROM [{old_table}]".format( new_table=new_table_name, old_table=self.name, old_cols=", ".join("[{}]".format(col) for col in columns_to_copy), new_cols=", ".join("[{}]".format(rename.get(col, col)) for col in columns_to_copy), ) self.db.conn.execute(copy_sql) # Drop the old table self.db.co… {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
655786374 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-655786374 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NTc4NjM3NA== simonw 9599 2020-07-08T22:16:54Z 2020-07-08T22:16:54Z OWNER According to https://www.sqlite.org/lang_altertable.html#making_other_kinds_of_table_schema_changes the hardest bits to consider are how to deal with existing foreign key relationships, triggers and views. I'm OK leaving views as an exercise for the caller - many of these transformations may not need any view changes at all. Foreign key relationships are important: it should handle these automatically as effectively as possible. Likewise trigger changes: need to think about what this means. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
656363548 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-656363548 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY1NjM2MzU0OA== simonw 9599 2020-07-09T21:37:28Z 2020-07-09T21:37:28Z OWNER I'm going to add a second method `.transform_table_sql(...)` - which returns the SQL that would have been executed but does NOT execute it. Advanced callers can use this to include their own additional steps in the same transaction - e.g. recreating views or triggers. More importantly it gives me a useful hook for writing some unit tests against the generated SQL. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
664106621 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-664106621 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY2NDEwNjYyMQ== simonw 9599 2020-07-27T04:01:13Z 2020-07-27T04:01:13Z OWNER Work in progress in `transform` branch here: https://github.com/simonw/sqlite-utils/tree/transform {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696421240 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696421240 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQyMTI0MA== simonw 9599 2020-09-21T22:53:48Z 2020-09-21T22:53:48Z OWNER I've decided to call this `table.transform()` - I was over-thinking whether people would remember that `.transform()` actually transforms the table, but that's what documentation is for. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696423066 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696423066 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQyMzA2Ng== simonw 9599 2020-09-21T22:59:01Z 2020-09-21T22:59:01Z OWNER I'm rethinking the API design now. Maybe it could look like this: To change the type of the `author_id` column from `text` to `int`: ```python books.transform({"author_id": int}) ``` This would leave the existing columns alone, but would change the type of this column. To rename `author_id` to `author_identifier`: ```python books.transform(rename={"author_id": "author_identifier"}) ``` To drop a column: ```python books.transform(drop=["author_id"]) ``` Since the parameters all operate on columns they don't need to be called `drop_column` and `rename_column`. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696423138 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696423138 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQyMzEzOA== simonw 9599 2020-09-21T22:59:17Z 2020-09-21T23:01:06Z OWNER I'm going to sketch out a prototype of this new API design in that branch. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696430843 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696430843 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzMDg0Mw== simonw 9599 2020-09-21T23:21:00Z 2020-09-21T23:21:00Z OWNER For FTS tables associated with the table that is being transformed, should I automatically drop the old FTS table and recreate it against the new one or will it just magically continue to work after the table is renamed? {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696431058 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696431058 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzMTA1OA== simonw 9599 2020-09-21T23:21:37Z 2020-09-21T23:21:37Z OWNER I may need to do something special for `rowid` tables to ensure that the `rowid` values in the transformed table match those from the old table. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696432690 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696432690 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzMjY5MA== simonw 9599 2020-09-21T23:26:32Z 2020-09-21T23:27:38Z OWNER To expand on what that first argument - the `columns` argument - does. Say you have a table like this: ``` id integer name text age text ``` Any columns omitted from the `columns=` argument are left alone - they have to be explicitly dropped using `drop=` if you want to drop them. Any new columns are added (at the end of the table): ``` table.tranform({"size": float}) ``` Any columns that have their type changed will have their type changed: ``` table.tranform({"age": int}) ``` Should I also re-order columns if the order doesn't match? I think so. Open question as to what happens to columns that aren't mentioned at all in the dictionary though - what order should they go in? {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696433542 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696433542 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzMzU0Mg== simonw 9599 2020-09-21T23:28:58Z 2020-09-21T23:28:58Z OWNER If you want to both change the type of a column AND rename it in the same operation, how would you do that? I think like this: ```python table.transform({"age": int}, rename={"age": "dog_age"}) ``` So any rename logic is applied at the end, after the type transformation or re-ordering logic. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696433778 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696433778 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzMzc3OA== simonw 9599 2020-09-21T23:29:39Z 2020-09-21T23:29:39Z OWNER The `columns=` argument is optional - so you can do just a rename operation like so: ``` table.transform(rename={"age": "dog_age"}) ``` {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696434097 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696434097 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzNDA5Nw== simonw 9599 2020-09-21T23:30:40Z 2020-09-21T23:30:40Z OWNER Since I have a `column_order=None` argument already, maybe I can ignore the order of the columns in that first argument and use that instead? {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696434237 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696434237 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzNDIzNw== simonw 9599 2020-09-21T23:31:07Z 2020-09-21T23:31:57Z OWNER Does it make sense to support the `pk=` argument for changing the primary key? If the user requests a primary key that doesn't make sense I think an integrity error will be raised when the SQL is being executed, which should hopefully cancel the transaction and raise an error. Need to check that this is what happens. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696434638 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696434638 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzNDYzOA== simonw 9599 2020-09-21T23:32:26Z 2020-09-21T23:32:26Z OWNER A test that confirms that this mechanism can turn a `rowid` into a non-rowid table would be good too. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696435194 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696435194 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQzNTE5NA== simonw 9599 2020-09-21T23:34:14Z 2020-09-21T23:35:00Z OWNER I think the fiddliest part of the implementation here is code that takes the existing `columns_dict` of the table and the incoming `columns=` and `drop=` and `rename=` parameters and produces the columns dictionary for the new table, ready to be fed to `.create_table()`. This logic probably also needs to return a structure that can be used to build the `INSERT INTO ... SELECT ... FROM` query. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696454485 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696454485 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjQ1NDQ4NQ== simonw 9599 2020-09-22T00:42:35Z 2020-09-22T00:42:35Z OWNER The reason I'm working on this now is that I'd like to support many more options for data cleanup in the Datasette ecosystem - so being able to do things like convert the type of existing columns becomes increasingly important. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  
696500767 https://github.com/simonw/sqlite-utils/issues/114#issuecomment-696500767 https://api.github.com/repos/simonw/sqlite-utils/issues/114 MDEyOklzc3VlQ29tbWVudDY5NjUwMDc2Nw== simonw 9599 2020-09-22T04:21:45Z 2020-09-22T04:21:45Z OWNER Documentation: https://sqlite-utils.readthedocs.io/en/latest/python-api.html#transforming-a-table {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} table.transform() method for advanced alter table 621989740  

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options: 

CREATE TABLE [issue_comments] (
   [html_url] TEXT,
   [issue_url] TEXT,
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [created_at] TEXT,
   [updated_at] TEXT,
   [author_association] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [issue] INTEGER REFERENCES [issues]([id])
, [performed_via_github_app] TEXT);
CREATE INDEX [idx_issue_comments_issue]
                ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
                ON [issue_comments] ([user]);