[GitHub] [spark] cloud-fan commented on a change in pull request #27006: [SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function

2020-05-22 Thread GitBox 


cloud-fan commented on a change in pull request #27006:
URL: https://github.com/apache/spark/pull/27006#discussion_r429181892



##
File path: sql/core/src/test/resources/sql-tests/inputs/misc-functions.sql
##
@@ -8,3 +8,6 @@ select typeof(cast(1.0 as float)), typeof(1.0D), typeof(1.2);
 select typeof(date '1986-05-23'),  typeof(timestamp '1986-05-23'), 
typeof(interval '23 days');
 select typeof(x'ABCD'), typeof('SPARK');
 select typeof(array(1, 2)), typeof(map(1, 2)), typeof(named_struct('a', 1, 
'b', 'spark'));
+
+-- get current_catalog
+select current_catalog();

Review comment:
   how about we add a `current_database_catalog.sql` and test both of them 
there?





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #27006: [SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function

2020-05-22 Thread GitBox 


cloud-fan commented on a change in pull request #27006:
URL: https://github.com/apache/spark/pull/27006#discussion_r429136494



##
File path: sql/core/src/test/resources/sql-tests/inputs/misc-functions.sql
##
@@ -8,3 +8,6 @@ select typeof(cast(1.0 as float)), typeof(1.0D), typeof(1.2);
 select typeof(date '1986-05-23'),  typeof(timestamp '1986-05-23'), 
typeof(interval '23 days');
 select typeof(x'ABCD'), typeof('SPARK');
 select typeof(array(1, 2)), typeof(map(1, 2)), typeof(named_struct('a', 1, 
'b', 'spark'));
+
+-- get current_catalog
+select current_catalog();

Review comment:
   do we have a sql testing file for `current_database`?





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #27006: [SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function

2020-05-22 Thread GitBox 


cloud-fan commented on a change in pull request #27006:
URL: https://github.com/apache/spark/pull/27006#discussion_r429136168



##
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/finishAnalysis.scala
##
@@ -91,15 +91,21 @@ object ComputeCurrentTime extends Rule[LogicalPlan] {
 }
 
 
-/** Replaces the expression of CurrentDatabase with the current database name. 
*/
-case class GetCurrentDatabase(catalogManager: CatalogManager) extends 
Rule[LogicalPlan] {
+/**
+ * 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] {
   def apply(plan: LogicalPlan): LogicalPlan = {
 import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
-val currentNamespace = catalogManager.currentNamespace.quoted
 
 plan transformAllExpressions {
   case CurrentDatabase() =>
+val currentNamespace = catalogManager.currentNamespace.quoted

Review comment:
   it's better to get the value before `transformAllExpressions`

##
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/finishAnalysis.scala
##
@@ -91,15 +91,21 @@ object ComputeCurrentTime extends Rule[LogicalPlan] {
 }
 
 
-/** Replaces the expression of CurrentDatabase with the current database name. 
*/
-case class GetCurrentDatabase(catalogManager: CatalogManager) extends 
Rule[LogicalPlan] {
+/**
+ * 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] {
   def apply(plan: LogicalPlan): LogicalPlan = {
 import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
-val currentNamespace = catalogManager.currentNamespace.quoted
 
 plan transformAllExpressions {
   case CurrentDatabase() =>
+val currentNamespace = catalogManager.currentNamespace.quoted
 Literal.create(currentNamespace, StringType)
+  case CurrentCatalog() =>
+val currentCatalog = catalogManager.currentCatalog.name()

Review comment:
   ditto





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #27006: [SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function

2020-05-22 Thread GitBox 


cloud-fan commented on a change in pull request #27006:
URL: https://github.com/apache/spark/pull/27006#discussion_r429135679



##
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/finishAnalysis.scala
##
@@ -91,15 +91,21 @@ object ComputeCurrentTime extends Rule[LogicalPlan] {
 }
 
 
-/** Replaces the expression of CurrentDatabase with the current database name. 
*/
-case class GetCurrentDatabase(catalogManager: CatalogManager) extends 
Rule[LogicalPlan] {
+/**
+ * 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] {

Review comment:
   `GetCurrentDatebaseAndCatalog`





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #27006: [SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function

2020-05-22 Thread GitBox 


cloud-fan commented on a change in pull request #27006:
URL: https://github.com/apache/spark/pull/27006#discussion_r429135447



##
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala
##
@@ -116,6 +116,24 @@ case class CurrentDatabase() extends LeafExpression with 
Unevaluable {
   override def prettyName: String = "current_database"
 }
 
+/**
+ * Returns the current catalog.
+ */
+@ExpressionDescription(
+  usage = "_FUNC_() - Returns the current catalog.",
+  examples = """
+Examples:
+  > SELECT _FUNC_();
+   spark_catalog
+  """,
+  since = "3.0.0")

Review comment:
   it's too late for 3.0 now. let's change it to `3.1.0`





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #27006: [SPARK-30352][SQL] DataSourceV2: Add CURRENT_CATALOG function

2020-05-20 Thread GitBox 


cloud-fan commented on a change in pull request #27006:
URL: https://github.com/apache/spark/pull/27006#discussion_r427807877



##
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
##
@@ -130,6 +130,7 @@ abstract class Optimizer(catalogManager: CatalogManager)
   ReplaceExpressions,
   ComputeCurrentTime,
   GetCurrentDatabase(catalogManager),
+  GetCurrentCatalog(catalogManager),

Review comment:
   can we merge the above 2 rules into one?





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org