Refine single-column Table to act as a Column #12165
Conversation
radeusgd
added
the
CI: No changelog needed
…sure the copy-pasted code is not buggy...
radeusgd
force-pushed
the
wip/radeusgd/12137-table-as-single-column
branch
from
03cd724 to
7be3f68
Compare
radeusgd
requested review from
jdunkerley,
GregoryTravis,
AdRiley and
marthasharkey
as code owners
| result.catch SQL_Error _-> | ||
| if result.is_error then | ||
| Error.throw (Table_Not_Found.Error name) | ||
| result |
There was a problem hiding this comment.
Since #11777 we can simplify the error handling code, let's use it :)
| ## Converts all table contents to a text value in delimited format. | ||
|
|
||
| Arguments: | ||
| - format: Allows to customize the delimiter and other settings of the | ||
| format. Defaults to tab-separated values. | ||
| to_delimited self (format:Delimited_Format = ..Delimited '\t') -> Text = | ||
| Delimited_Writer.write_text self format |
There was a problem hiding this comment.
We have agreed that Text.from Table conversion wasn't that useful and so we are turning it into a method on Table instead.
Does that name seem good or any suggestions for a better one?
| ## Converts all table contents to a text value in delimited format. | ||
|
|
||
| Arguments: | ||
| - format: Allows to customize the delimiter and other settings of the | ||
| format. Defaults to tab-separated values. | ||
| to_delimited self (format:Delimited_Format = ..Delimited '\t') -> Text = | ||
| Delimited_Writer.write_text self format |
| refine_table (table : Table) = | ||
| if is_single_column table . not then table else | ||
| column = table.at 0 | ||
| # This should be consistent with `Column_Refinements.refine_column` - the code needs to be copied because we need to spell out all the types. |
There was a problem hiding this comment.
Probably one we need to talk over with Jaroslav to see if we can do anything better longer term.
| Value_Type.Time -> (table : Table & Column & Time_Of_Day) | ||
| Value_Type.Date_Time True -> (table : Table & Column & Date_Time) | ||
| Value_Type.Decimal _ scale -> | ||
| is_integer = scale == 0 |
There was a problem hiding this comment.
What about scale < 0? It can still be an integer, so would Table & Column & Integer be the right type in that case?
| @@ -77,26 +77,26 @@ _merge_input_and_tables (input_table : Table) (tables_for_rows : Vector Read_Man | |||
|
|
|||
| multiplicated_inputs = duplicate_rows input_table counts | |||
| Runtime.assert (unified_data.row_count == multiplicated_inputs.row_count) | |||
| Runtime.assert (unified_metadata.is_nothing || (unified_metadata.row_count == unified_data.row_count)) | |||
| Runtime.assert ((Nothing == unified_metadata) || (unified_metadata.row_count == unified_data.row_count)) | |||
There was a problem hiding this comment.
Why is this change needed?
| t1 = setup.table_builder [["A", [23]]] | ||
| t1.should_be_a DB_Table | ||
| (t1:DB_Column).name.should_equal "A" | ||
| Test.expect_panic Type_Error (t1:Integer).to_vector |
There was a problem hiding this comment.
I am confused by this test. to_vector is not a method of Integer, so I would expect a method-not-found error here.
| ## The internal constructor used to construct a DB_Table instance. | ||
|
|
||
| It can perform some additional operations, like refining the type, | ||
| so it should always be preferred over calling `DB_Table.Value` directly. |
There was a problem hiding this comment.
How would I know this without reading this comment? Guess we need some sort of convention for module private constructors similar to what we have for module priavte methods
There was a problem hiding this comment.
Seeing it in use has led me to a new question. (And I think this is the question that has been in my head all week trying to get out. :))
If the types are hidden then how are they any use? And I think the answer we have been saying is: well they can be cast to the hidden types. And the GUI will cast them to the hidden types. But how will the GUI know it can cast them to the hidden types if they are hidden?
Pull Request Description
Tablethat has just 1 column to act asColumn#12137Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.