You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/07/01 03:19:38 UTC

[GitHub] [spark] zhengruifeng opened a new pull request, #37039: [SPARK-39649][PYTHON][WIP] Make *Database* / listColumns / refreshTable in pyspark support 3-layer-namespace

zhengruifeng opened a new pull request, #37039:
URL: https://github.com/apache/spark/pull/37039

   ### What changes were proposed in this pull request?
   
   make following commands in pyspark support 3-layer-namespace
   
   - setCurrentDatabase (WIP)
   - listDatabases
   - getDatabase
   - listColumns
   - refreshTable
   
   
   
   ### Why are the changes needed?
   to support 3-layer-namespace
   
   
   ### Does this PR introduce _any_ user-facing change?
   yes, new api added
   
   ### How was this patch tested?
   updated UT


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] amaliujia commented on a diff in pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #37039:
URL: https://github.com/apache/spark/pull/37039#discussion_r912318060


##########
python/pyspark/sql/tests/test_catalog.py:
##########
@@ -53,6 +53,14 @@ def test_database_exists(self):
             self.assertTrue(spark.catalog.databaseExists("spark_catalog.some_db"))
             self.assertFalse(spark.catalog.databaseExists("spark_catalog.some_db2"))
 
+    def test_get_database(self):

Review Comment:
   No need to add a freshtable API test on Python side?



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] cloud-fan closed pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace
URL: https://github.com/apache/spark/pull/37039


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] zhengruifeng commented on pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #37039:
URL: https://github.com/apache/spark/pull/37039#issuecomment-1172810479

   cc @cloud-fan @amaliujia 


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] zhengruifeng commented on pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #37039:
URL: https://github.com/apache/spark/pull/37039#issuecomment-1173244557

   also cc @HyukjinKwon if you have some time


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] amaliujia commented on a diff in pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #37039:
URL: https://github.com/apache/spark/pull/37039#discussion_r912317990


##########
python/pyspark/sql/catalog.py:
##########
@@ -139,11 +140,40 @@ def listDatabases(self) -> List[Database]:
             jdb = iter.next()
             databases.append(
                 Database(
-                    name=jdb.name(), description=jdb.description(), locationUri=jdb.locationUri()
+                    name=jdb.name(),
+                    catalog=jdb.catalog(),
+                    description=jdb.description(),
+                    locationUri=jdb.locationUri(),
                 )
             )
         return databases
 
+    def getDatabase(self, dbName: str) -> Database:
+        """Get the database with the specified name.
+        This throws an AnalysisException when the database cannot be found.
+
+        .. versionadded:: 3.4.0
+
+        Parameters
+        ----------
+        dbName : str
+             name of the database to check existence.
+
+        Examples
+        --------
+        >>> spark.catalog.getDatabase("default")
+        Database(name='default', catalog=None, description='default database', ...

Review Comment:
   I was not aware of this though:
   
   Is the `catalog=None` because of current catalog is not set? I am asking because I thought every DB should have a catalog.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] zhengruifeng commented on a diff in pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #37039:
URL: https://github.com/apache/spark/pull/37039#discussion_r912570264


##########
python/pyspark/sql/tests/test_catalog.py:
##########
@@ -53,6 +53,14 @@ def test_database_exists(self):
             self.assertTrue(spark.catalog.databaseExists("spark_catalog.some_db"))
             self.assertFalse(spark.catalog.databaseExists("spark_catalog.some_db2"))
 
+    def test_get_database(self):

Review Comment:
   done



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] zhengruifeng commented on a diff in pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #37039:
URL: https://github.com/apache/spark/pull/37039#discussion_r912318495


##########
python/pyspark/sql/catalog.py:
##########
@@ -139,11 +140,40 @@ def listDatabases(self) -> List[Database]:
             jdb = iter.next()
             databases.append(
                 Database(
-                    name=jdb.name(), description=jdb.description(), locationUri=jdb.locationUri()
+                    name=jdb.name(),
+                    catalog=jdb.catalog(),
+                    description=jdb.description(),
+                    locationUri=jdb.locationUri(),
                 )
             )
         return databases
 
+    def getDatabase(self, dbName: str) -> Database:
+        """Get the database with the specified name.
+        This throws an AnalysisException when the database cannot be found.
+
+        .. versionadded:: 3.4.0
+
+        Parameters
+        ----------
+        dbName : str
+             name of the database to check existence.
+
+        Examples
+        --------
+        >>> spark.catalog.getDatabase("default")
+        Database(name='default', catalog=None, description='default database', ...

Review Comment:
   because of the old code path, which intilize a `Database` with `null` catalog:
   
   https://github.com/apache/spark/pull/36968/files#diff-d7036fd31ee9734405f0e877b79e36675d567b6f9acb844034383463e80a54ccR65-R67



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] cloud-fan commented on a diff in pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37039:
URL: https://github.com/apache/spark/pull/37039#discussion_r912980055


##########
python/pyspark/sql/catalog.py:
##########
@@ -139,11 +140,40 @@ def listDatabases(self) -> List[Database]:
             jdb = iter.next()
             databases.append(
                 Database(
-                    name=jdb.name(), description=jdb.description(), locationUri=jdb.locationUri()
+                    name=jdb.name(),
+                    catalog=jdb.catalog(),
+                    description=jdb.description(),
+                    locationUri=jdb.locationUri(),
                 )
             )
         return databases
 
+    def getDatabase(self, dbName: str) -> Database:
+        """Get the database with the specified name.
+        This throws an AnalysisException when the database cannot be found.
+
+        .. versionadded:: 3.4.0
+
+        Parameters
+        ----------
+        dbName : str
+             name of the database to check existence.
+
+        Examples
+        --------
+        >>> spark.catalog.getDatabase("default")
+        Database(name='default', catalog=None, description='default database', ...

Review Comment:
   let's fix this at scala side as well later.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] cloud-fan commented on pull request #37039: [SPARK-39649][PYTHON] Make listDatabases / getDatabase / listColumns / refreshTable in PySpark support 3-layer-namespace

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37039:
URL: https://github.com/apache/spark/pull/37039#issuecomment-1173792353

   thanks, merging to master!


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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