-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function #27006
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
Conversation
|
Test build #115761 has finished for PR 27006 at commit
|
|
retest this please |
|
Test build #115763 has finished for PR 27006 at commit
|
|
retest this please |
|
Test build #115840 has finished for PR 27006 at commit
|
|
retest this please, and thanks @HyukjinKwon |
|
Test build #115850 has finished for PR 27006 at commit
|
|
Test build #121870 has finished for PR 27006 at commit
|
|
cc @cloud-fan FYI |
|
retest this please |
| ReplaceExpressions, | ||
| ComputeCurrentTime, | ||
| GetCurrentDatabase(catalogManager), | ||
| GetCurrentCatalog(catalogManager), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we merge the above 2 rules into one?
|
Test build #122878 has finished for PR 27006 at commit
|
|
Test build #122889 has finished for PR 27006 at commit
|
| > SELECT _FUNC_(); | ||
| spark_catalog | ||
| """, | ||
| since = "3.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's too late for 3.0 now. let's change it to 3.1.0
| * Replaces the expression of CurrentDatabase with the current database name. | ||
| * Replaces the expression of CurrentCatalog with the current catalog name. | ||
| */ | ||
| case class GetCurrentDatabaseOrCatalog(catalogManager: CatalogManager) extends Rule[LogicalPlan] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetCurrentDatebaseAndCatalog
|
|
||
| plan transformAllExpressions { | ||
| case CurrentDatabase() => | ||
| val currentNamespace = catalogManager.currentNamespace.quoted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to get the value before transformAllExpressions
| val currentNamespace = catalogManager.currentNamespace.quoted | ||
| Literal.create(currentNamespace, StringType) | ||
| case CurrentCatalog() => | ||
| val currentCatalog = catalogManager.currentCatalog.name() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
| select typeof(array(1, 2)), typeof(map(1, 2)), typeof(named_struct('a', 1, 'b', 'spark')); | ||
|
|
||
| -- get current_catalog | ||
| select current_catalog(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have a sql testing file for current_database?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked and not found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about we add a current_database_catalog.sql and test both of them there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am okay with it.
|
Test build #122991 has finished for PR 27006 at commit
|
|
sorry it conflicts, can you fix it? |
|
Test build #122996 has finished for PR 27006 at commit
|
|
Test build #123007 has finished for PR 27006 at commit
|
|
retest this please |
1 similar comment
|
retest this please |
|
Test build #123024 has finished for PR 27006 at commit
|
|
retest this please |
|
Test build #123032 has finished for PR 27006 at commit
|
|
thanks, merging to master! |
What changes were proposed in this pull request?
As we support multiple catalogs with DataSourceV2, we may need the
CURRENT_CATALOGvalue expression from the SQL standard.CURRENT_CATALOGis a general value specification in the SQL Standard, described as:Why are the changes needed?
improve catalog v2 with ANSI SQL standard.
Does this PR introduce any user-facing change?
yes, add a new function
current_catalog()to point the current active catalogHow was this patch tested?
add ut