Skip to content
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

Saving data links to a DB_Table #11371

Merged
merged 32 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8e37018
(de)serialization of SQL_Statement
radeusgd Oct 17, 2024
787530f
WIP saving as data link
radeusgd Oct 17, 2024
86d0b9d
updating schema
radeusgd Oct 17, 2024
6110c05
enable Cloud auth in Snowflake tests, to be able to test the data lin…
radeusgd Oct 18, 2024
cfec370
regenerate workflow after change
radeusgd Oct 18, 2024
fd643ca
checkpoint - interpreting datalinks, creating tables from SQL_Statement
radeusgd Oct 18, 2024
fb743cf
add custom tests for Postgres
radeusgd Oct 21, 2024
10eed0f
add common tests for saving table/query as data link
radeusgd Oct 21, 2024
7e7ac9e
simplify Postgres datalink - now one constructor with link type
radeusgd Oct 21, 2024
ff20d4b
parsing data link type
radeusgd Oct 21, 2024
5d5e609
fix compiler errors
radeusgd Oct 21, 2024
8011e7d
shorter table name, typo
radeusgd Oct 21, 2024
ec0d661
fixing
radeusgd Oct 21, 2024
1e23ec6
fix typo
radeusgd Oct 21, 2024
3e4cc58
fix
radeusgd Oct 21, 2024
cbd3f63
better names
radeusgd Oct 21, 2024
3f1c71a
integrate Snowflake and SQLServer
radeusgd Oct 21, 2024
97dbecf
add types - enable autoscoping for Case Sensitivity in Text
radeusgd Oct 21, 2024
b0e6bec
warn if temporary table is saved as data link
radeusgd Oct 21, 2024
ab02438
tests for the temp warning
radeusgd Oct 21, 2024
9b58c89
fix for various temp table styles
radeusgd Oct 21, 2024
f1e7b0a
test and fix edge case
radeusgd Oct 21, 2024
3fc114e
changelog
radeusgd Oct 21, 2024
e0efc3c
fix tests, more signatures
radeusgd Oct 22, 2024
1400bdc
further tweaking to is_trivial_query
radeusgd Oct 22, 2024
cae6fd8
more tweaking
radeusgd Oct 22, 2024
a768c0b
fix test
radeusgd Oct 22, 2024
bf4b8ef
check invariants, run Snowflake on GH to have aws cli
radeusgd Oct 22, 2024
5fec3c3
better encapsulation and responsibility delegation
radeusgd Oct 22, 2024
15edb3a
Merge branch 'develop' into wip/radeusgd/11295-save-table-as-datalink
radeusgd Oct 24, 2024
8e7d237
CR1
radeusgd Oct 24, 2024
034e490
CR2
radeusgd Oct 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/extra-nightly-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- run: ./run backend test std-snowflake
env:
ENSO_CLOUD_COGNITO_REGION: ${{ vars.ENSO_CLOUD_COGNITO_REGION }}
ENSO_CLOUD_COGNITO_USER_POOL_ID: ${{ vars.ENSO_CLOUD_COGNITO_USER_POOL_ID }}
ENSO_CLOUD_COGNITO_USER_POOL_WEB_CLIENT_ID: ${{ vars.ENSO_CLOUD_COGNITO_USER_POOL_WEB_CLIENT_ID }}
ENSO_CLOUD_TEST_ACCOUNT_PASSWORD: ${{ secrets.ENSO_CLOUD_TEST_ACCOUNT_PASSWORD }}
ENSO_CLOUD_TEST_ACCOUNT_USERNAME: ${{ secrets.ENSO_CLOUD_TEST_ACCOUNT_USERNAME }}
ENSO_SNOWFLAKE_ACCOUNT: ${{ secrets.ENSO_SNOWFLAKE_ACCOUNT }}
ENSO_SNOWFLAKE_DATABASE: ${{ secrets.ENSO_SNOWFLAKE_DATABASE }}
ENSO_SNOWFLAKE_PASSWORD: ${{ secrets.ENSO_SNOWFLAKE_PASSWORD }}
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
cloud.][11235]
- [The user may set description and labels of an Enso Cloud asset
programmatically.][11255]
- [DB_Table may be saved as a Data Link.][11371]

[11235]: https://github.com/enso-org/enso/pull/11235
[11255]: https://github.com/enso-org/enso/pull/11255
[11371]: https://github.com/enso-org/enso/pull/11371

# Enso 2024.4

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,12 @@ v.test('correctly validates example Table .datalink files with the schema', () =
})

v.test('correctly validates example Database .datalink files with the schema', () => {
const schemas = ['postgres-db.datalink', 'postgres-table.datalink']
const schemas = [
'postgres-db.datalink',
'postgres-table.datalink',
'postgres-simple-query.datalink',
'postgres-serialized-query.datalink',
]
for (const schema of schemas) {
const json = loadDataLinkFile(path.resolve(TABLE_DATA_LINKS_ROOT, schema))
testSchema(json, schema)
Expand Down
35 changes: 32 additions & 3 deletions app/gui/src/dashboard/data/datalinkSchema.json
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
},
"required": ["username", "password"]
},
"table": { "title": "Table to access", "type": "string" }
"table": { "title": "Table to access", "$ref": "#/$defs/DatabaseTableDefinition" }
},
"required": ["type", "libraryName", "host", "port", "database_name"]
},
Expand Down Expand Up @@ -233,7 +233,7 @@
},
"required": ["username", "password"]
},
"table": { "title": "Table to access", "type": "string" }
"table": { "title": "Table to access", "$ref": "#/$defs/DatabaseTableDefinition" }
},
"required": ["type", "libraryName", "account", "database_name", "credentials"]
},
Expand Down Expand Up @@ -277,7 +277,7 @@
},
"required": ["username", "password"]
},
"table": { "title": "Table to access", "type": "string" }
"table": { "title": "Table to access", "$ref": "#/$defs/DatabaseTableDefinition" }
},
"required": ["type", "libraryName", "host", "port", "database_name"]
},
Expand Down Expand Up @@ -458,6 +458,35 @@
}
},
"required": ["type", "subType"]
},

"DatabaseTableDefinition": {
"title": "Query to read",
"anyOf": [
{ "title": "Table", "type": "string" },
{
"title": "SQL Query",
"type": "object",
"properties": {
"query": {
"title": "SQL",
"type": "string"
}
},
"required": ["query"]
},
{
"title": "(Advanced) JSON serialized interpolated SQL statement",
"type": "object",
"properties": {
"sql_statement": {
"title": "SQL_Statement (JSON)",
"type": "string"
}
},
"required": ["sql_statement"]
}
]
}
}
}
9 changes: 4 additions & 5 deletions build/build/src/ci_gen/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,13 +349,12 @@ impl JobArchetype for SnowflakeTests {
crate::libraries_tests::snowflake::env::ENSO_SNOWFLAKE_WAREHOUSE,
);

// Temporarily disabled until we can get the Cloud auth fixed.
// Snowflake does not rely on cloud anyway, so it can be disabled.
// But it will rely once we add datalink tests, so this should be fixed soon.
// let updated_main_step = enable_cloud_tests(main_step);
// Snowflake tests are run only in the 'Extra' job, so it is okay to run it with
// Enso Cloud as well. They need it to test data link integration.
let updated_main_step = enable_cloud_tests(main_step);

vec![
main_step,
updated_main_step,
step::extra_stdlib_test_reporter(target, GRAAL_EDITION_FOR_EXTRA_TESTS),
]
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ Text.characters self =
## This matches `aBc` @ character 11
"aabbbbccccaaBcaaaa".find "a[ab]c" Case_Sensitivity.Insensitive
Text.find : (Regex | Text) -> Case_Sensitivity -> Match | Nothing ! Regex_Syntax_Error | Illegal_Argument
Text.find self pattern:(Regex | Text)=".*" case_sensitivity=Case_Sensitivity.Sensitive =
Text.find self pattern:(Regex | Text)=".*" case_sensitivity:Case_Sensitivity=..Sensitive =
case_insensitive = case_sensitivity.is_case_insensitive_in_memory
compiled_pattern = Regex.compile pattern case_insensitive=case_insensitive
compiled_pattern.match self
Expand Down Expand Up @@ -300,7 +300,7 @@ Text.find self pattern:(Regex | Text)=".*" case_sensitivity=Case_Sensitivity.Sen
## This matches `aABbbbc` @ character 0 and `aBC` @ character 11
"aABbbbccccaaBCaaaa".find_all "a[ab]+c" Case_Sensitivity.Insensitive
Text.find_all : Text -> Case_Sensitivity -> Vector Match ! Regex_Syntax_Error | Illegal_Argument
Text.find_all self pattern=".*" case_sensitivity=Case_Sensitivity.Sensitive =
Text.find_all self pattern=".*" case_sensitivity:Case_Sensitivity=..Sensitive =
case_insensitive = case_sensitivity.is_case_insensitive_in_memory
compiled_pattern = Regex.compile pattern case_insensitive=case_insensitive
compiled_pattern.match_all self
Expand Down Expand Up @@ -335,7 +335,7 @@ Text.find_all self pattern=".*" case_sensitivity=Case_Sensitivity.Sensitive =
# Evaluates to true
"[email protected]".match regex Case_Sensitivity.Insensitive
Text.match : Text -> Case_Sensitivity -> Boolean ! Regex_Syntax_Error | Illegal_Argument
Text.match self pattern=".*" case_sensitivity=Case_Sensitivity.Sensitive =
Text.match self pattern=".*" case_sensitivity:Case_Sensitivity=..Sensitive =
case_insensitive = case_sensitivity.is_case_insensitive_in_memory
compiled_pattern = Regex.compile pattern case_insensitive=case_insensitive
compiled_pattern.matches self
Expand Down Expand Up @@ -394,7 +394,7 @@ Text.to_regex self case_insensitive=False = Regex.compile self case_insensitive
'azbzczdzezfzg'.split ['b', 'zez'] == ['az', 'zczd', 'fzg']
@delimiter make_delimiter_selector
Text.split : Text | Vector Text -> Case_Sensitivity -> Boolean -> Vector Text ! Illegal_Argument
Text.split self delimiter="," case_sensitivity=Case_Sensitivity.Sensitive use_regex=False =
Text.split self delimiter="," case_sensitivity:Case_Sensitivity=..Sensitive use_regex=False =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Text.split self delimiter="," case_sensitivity:Case_Sensitivity=..Sensitive use_regex=False =
Text.split self delimiter="," case_sensitivity:Case_Sensitivity=..Sensitive use_regex:Boolean=False =

We should get rid of use_regex and take Regex or Text.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an out of scope change for this PR IMHO. Maybe I shouldn't have started the type changes at all - but I just wanted to do the trivial ones and thought they could be done 'in the meantime'.

But this requires logical changes and changes to the tests, I'd prefer to do it separately.

delimiter_is_empty = case delimiter of
_ : Text -> delimiter.is_empty
_ : Vector -> delimiter.is_empty || delimiter.any (.is_empty)
Expand Down Expand Up @@ -903,7 +903,7 @@ Text.from_codepoints codepoints = Text_Utils.from_codepoints codepoints
"Hello!".starts_with "hello" == False
"Hello!".starts_with "hello" Case_Sensitivity.Insensitive == True
Text.starts_with : Text -> Case_Sensitivity -> Boolean
Text.starts_with self prefix case_sensitivity=Case_Sensitivity.Sensitive = case case_sensitivity of
Text.starts_with self prefix case_sensitivity:Case_Sensitivity=..Sensitive = case case_sensitivity of
Case_Sensitivity.Default -> self.starts_with prefix Case_Sensitivity.Sensitive
Case_Sensitivity.Sensitive -> Text_Utils.starts_with self prefix
Case_Sensitivity.Insensitive locale ->
Expand Down Expand Up @@ -933,7 +933,7 @@ Text.starts_with self prefix case_sensitivity=Case_Sensitivity.Sensitive = case
"Hello World".ends_with "world" == False
"Hello World".ends_with "world" Case_Sensitivity.Insensitive == True
Text.ends_with : Text -> Case_Sensitivity -> Boolean
Text.ends_with self suffix case_sensitivity=Case_Sensitivity.Sensitive = case case_sensitivity of
Text.ends_with self suffix case_sensitivity:Case_Sensitivity=..Sensitive = case case_sensitivity of
Case_Sensitivity.Default -> self.ends_with suffix Case_Sensitivity.Sensitive
Case_Sensitivity.Sensitive -> Text_Utils.ends_with self suffix
Case_Sensitivity.Insensitive locale ->
Expand Down Expand Up @@ -979,7 +979,7 @@ Text.ends_with self suffix case_sensitivity=Case_Sensitivity.Sensitive = case ca

"Hello!".contains "LO" Case_Sensitivity.Insensitive
Text.contains : Text -> Case_Sensitivity -> Boolean
Text.contains self term="" case_sensitivity=Case_Sensitivity.Sensitive = case case_sensitivity of
Text.contains self term:Text="" case_sensitivity:Case_Sensitivity=..Sensitive = case case_sensitivity of
Case_Sensitivity.Default -> self.contains term Case_Sensitivity.Sensitive
Case_Sensitivity.Sensitive -> Text_Utils.contains self term
Case_Sensitivity.Insensitive locale ->
Expand Down Expand Up @@ -1343,7 +1343,7 @@ Text.trim self where:Location=..Both what=_.is_whitespace =
match_1 == match_2

Text.locate : Text -> Matching_Mode -> Case_Sensitivity -> Span | Nothing
Text.locate self term="" mode=Matching_Mode.First case_sensitivity=Case_Sensitivity.Sensitive = case case_sensitivity of
Text.locate self term:Text="" mode=Matching_Mode.First case_sensitivity:Case_Sensitivity=..Sensitive = case case_sensitivity of
Case_Sensitivity.Default -> self.locate term mode Case_Sensitivity.Sensitive
Case_Sensitivity.Sensitive ->
codepoint_span = case mode of
Expand Down Expand Up @@ -1434,7 +1434,7 @@ Text.locate self term="" mode=Matching_Mode.First case_sensitivity=Case_Sensitiv
match_2 = ligatures . locate_all "ffiff" case_sensitivity=Case_Sensitive.Insensitive
match_2 . map .length == [2, 5]
Text.locate_all : Text -> Case_Sensitivity -> Vector Span
Text.locate_all self term="" case_sensitivity=Case_Sensitivity.Sensitive = if term.is_empty then Vector.new (self.length + 1) (ix -> Span.Value (ix.up_to ix) self) else case case_sensitivity of
Text.locate_all self term:Text="" case_sensitivity:Case_Sensitivity=..Sensitive = if term.is_empty then Vector.new (self.length + 1) (ix -> Span.Value (ix.up_to ix) self) else case case_sensitivity of
Case_Sensitivity.Default -> self.locate term Case_Sensitivity.Sensitive
Case_Sensitivity.Sensitive ->
codepoint_spans = Vector.from_polyglot_array <| Text_Utils.span_of_all self term
Expand Down Expand Up @@ -1479,7 +1479,7 @@ Text.locate_all self term="" case_sensitivity=Case_Sensitivity.Sensitive = if te
"Hello World!".index_of "J" == Nothing
"Hello World!".index_of "o" == 4
Text.index_of : Text -> Integer -> Case_Sensitivity -> Integer | Nothing
Text.index_of self term="" (start : Integer = 0) case_sensitivity=Case_Sensitivity.Sensitive =
Text.index_of self term:Text="" (start : Integer = 0) case_sensitivity:Case_Sensitivity=..Sensitive =
used_start = if start < 0 then start+self.length else start
if used_start < 0 || used_start > self.length then Error.throw (Index_Out_Of_Bounds.Error start self.length+1) else
used = if used_start == 0 then self else self.drop used_start
Expand Down Expand Up @@ -1514,7 +1514,7 @@ Text.index_of self term="" (start : Integer = 0) case_sensitivity=Case_Sensitivi
"Hello World!".last_index_of "J" == Nothing
"Hello World!".last_index_of "o" == 7
Text.last_index_of : Text -> Integer -> Case_Sensitivity -> Integer | Nothing
Text.last_index_of self term="" start=-1 case_sensitivity=Case_Sensitivity.Sensitive =
Text.last_index_of self term:Text="" start=-1 case_sensitivity:Case_Sensitivity=..Sensitive =
used_start = if start < 0 then start+self.length else start
if used_start < 0 || used_start >= self.length then Error.throw (Index_Out_Of_Bounds.Error start self.length) else
used = if used_start == self.length-1 then self else self.take used_start+1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import project.DB_Table as DB_Table_Module
import project.DB_Table.DB_Table
import project.Dialect.Dialect
import project.Internal.Connection.Entity_Naming_Properties.Entity_Naming_Properties
import project.Internal.Data_Link_Setup.Data_Link_Setup
import project.Internal.DDL_Transaction
import project.Internal.Hidden_Table_Registry
import project.Internal.In_Transaction.In_Transaction
Expand Down Expand Up @@ -59,7 +60,9 @@ type Connection
`False`.
- hidden_table_registry: a registry of hidden tables that are not
shown to the user, but are used internally by the dry-run system.
Value jdbc_connection dialect (entity_naming_properties : Entity_Naming_Properties) (supports_large_update : Ref Boolean) (hidden_table_registry : Hidden_Table_Registry.Hidden_Table_Registry)
- data_link_setup: an optional setup allowing for saving the connection
as a data link.
Value jdbc_connection dialect (entity_naming_properties : Entity_Naming_Properties) (supports_large_update : Ref Boolean) (hidden_table_registry : Hidden_Table_Registry.Hidden_Table_Registry) (data_link_setup : Data_Link_Setup | Nothing = Nothing)

## PRIVATE
Constructs a new Connection.
Expand All @@ -68,12 +71,15 @@ type Connection
- jdbc_connection: the resource managing the underlying JDBC
connection.
- dialect: the dialect associated with the database we are connected to.
- entity_naming_properties: a helper allowing to manage properties of
entity naming rules of the given backend.
- data_link_setup: an optional setup allowing for saving the connection
as a data link.
- try_large_update: whether the connection should try to use
`executeLargeUpdate`.
new : JDBC_Connection -> Dialect -> Entity_Naming_Properties -> Boolean -> Connection
new jdbc_connection dialect entity_naming_properties try_large_update=True =
new jdbc_connection:JDBC_Connection dialect entity_naming_properties:Entity_Naming_Properties (data_link_setup : Data_Link_Setup | Nothing = Nothing) (try_large_update : Boolean = True) -> Connection =
registry = Hidden_Table_Registry.new
Connection.Value jdbc_connection dialect entity_naming_properties (Ref.new try_large_update) registry
Connection.Value jdbc_connection dialect entity_naming_properties (Ref.new try_large_update) registry data_link_setup

## PRIVATE
Closes the connection releasing the underlying database resources
Expand Down Expand Up @@ -221,9 +227,7 @@ type Connection
make_table_for_name self name alias
SQL_Query.Raw_SQL raw_sql -> handle_sql_errors <| alias.if_not_error <|
self.dialect.ensure_query_has_no_holes self.jdbc_connection raw_sql . if_not_error <|
columns = self.fetch_columns raw_sql Statement_Setter.null
name = if alias == "" then (UUID.randomUUID.to_text) else alias
ctx = Context.for_query raw_sql name
r = make_table_from_query self raw_sql alias
## Any problems are treated as errors - e.g. if the query
contains clashing column names, it may very likely lead to
data corruption. Our renaming mechanism is used to fix issues
Expand All @@ -233,7 +237,6 @@ type Connection
will actually result in both columns `A` and `A 1` containing
the value 1; and value 2 being lost. That is why such queries
must fail.
r = DB_Table_Module.make_table self name columns ctx on_problems=Problem_Behavior.Report_Error
r.catch Any error->
Error.throw (Illegal_Argument.Error "The provided custom SQL query is invalid and may suffer data corruption when being processed, especially if it contains aliased column names, it may not be interpreted correctly. Please ensure the names are unique. The original error was: "+error.to_display_text cause=error)

Expand Down Expand Up @@ -476,6 +479,14 @@ type Connection
create_literal_table self (source : Table) (alias : Text) -> DB_Table =
DB_Table_Module.make_literal_table self (source.columns.map .to_vector) source.column_names alias

## ICON data_output
Creates a Data Link that will open the same connection.
@on_existing_file (Existing_File_Behavior.widget include_backup=False include_append=False)
save_as_data_link self destination (on_existing_file:Existing_File_Behavior = Existing_File_Behavior.Error) =
case self.data_link_setup of
Nothing -> Error.throw (Illegal_State.Error "This Database connection does not support saving it as a data link.")
data_link_setup -> data_link_setup.save_as_data_link destination on_existing_file


## PRIVATE
make_table_types_selector : Connection -> Widget
Expand Down Expand Up @@ -510,13 +521,25 @@ make_structure_creator =
item_editor = Single_Choice display=Display.Always values=[Option "new column" "(Column_Description.Value)"]
Vector_Editor item_editor=item_editor item_default=item_editor.values.first.value display=Display.Always

## PRIVATE
make_table_from_query connection query:Text|SQL_Statement alias:Text -> DB_Table =
expect_interpolations = case query of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect_interpolations = case query of
expect_interpolations = query.is_a Text

Type check will reject others.

# If the statement is given as a Text, then it should not contain `?` holes.
_ : Text -> False
_ : SQL_Statement -> True
statement_setter = if expect_interpolations then connection.dialect.get_statement_setter else Statement_Setter.null
columns = connection.base_connection.fetch_columns query statement_setter
name = if alias == "" then (UUID.randomUUID.to_text) else alias
ctx = Context.for_query query name
DB_Table_Module.make_table connection.base_connection name columns ctx on_problems=Problem_Behavior.Report_Error

## PRIVATE
make_table_for_name connection name alias internal_temporary_keep_alive_reference=Nothing =
result = handle_sql_errors <|
ctx = Context.for_table name (if alias == "" then name else alias) internal_temporary_keep_alive_reference
statement = connection.dialect.generate_sql (Query.Select Nothing ctx)
statement_setter = connection.dialect.get_statement_setter
columns = connection.fetch_columns statement statement_setter
columns = connection.base_connection.fetch_columns statement statement_setter
## In case of accessing an existing table, we assume that column names
are distinguishable by the backend, so any issues that are caught
only affect Enso columns, and just renaming Enso columns is enough to
Expand Down
Loading
Loading