You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/07/06 08:45:56 UTC

[GitHub] [incubator-superset] PU-101 opened a new pull request #10244: fix: error occurred when fetching large amount of columns

PU-101 opened a new pull request #10244:
URL: https://github.com/apache/incubator-superset/pull/10244


   ### SUMMARY
   when adding a table that has large amount of columns(ex: 1700), no column can be added to TableColumn.
   
   the specific cause is:
   (sqlite3.OperationalError) Expression tree is too large (maximum depth 1000)
   
   ### FIX
   change the filter condition to 'IN' which can reduce the number of params


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] villebro commented on a change in pull request #10244: fix: error occurred when fetching large amount of columns

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10244:
URL: https://github.com/apache/incubator-superset/pull/10244#discussion_r450588559



##########
File path: superset/connectors/sqla/models.py
##########
@@ -1188,12 +1188,16 @@ def fetch_metadata(self, commit: bool = True) -> None:
         any_date_col = None
         db_engine_spec = self.database.db_engine_spec
         db_dialect = self.database.get_dialect()
-        dbcols = (
-            db.session.query(TableColumn)
-            .filter(TableColumn.table == self)
-            .filter(or_(TableColumn.column_name == col.name for col in table_.columns))
-        )
-        dbcols = {dbcol.column_name: dbcol for dbcol in dbcols}
+        
+        try:
+            dbcols = (
+                db.session.query(TableColumn)
+                .filter(TableColumn.table == self)
+                .filter(TableColumn.column_name.in_([col.name for col in table.columns]))
+            )
+            dbcols = {dbcol.column_name: dbcol for dbcol in dbcols}
+        except Exception as e:
+            logger.error("Failed to get columns: " + str(e))

Review comment:
       What exception are we trying to catch here? In general we should always try to be as specific as possible as to what exception we are expecting to see, as opposed to catching all `Exception`.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] stale[bot] commented on pull request #10244: fix: error occurred when fetching large amount of columns

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #10244:
URL: https://github.com/apache/incubator-superset/pull/10244#issuecomment-687731352


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue.
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] stale[bot] closed pull request #10244: fix: error occurred when fetching large amount of columns

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #10244:
URL: https://github.com/apache/incubator-superset/pull/10244


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] PU-101 commented on a change in pull request #10244: fix: error occurred when fetching large amount of columns

Posted by GitBox <gi...@apache.org>.
PU-101 commented on a change in pull request #10244:
URL: https://github.com/apache/incubator-superset/pull/10244#discussion_r451354211



##########
File path: superset/connectors/sqla/models.py
##########
@@ -1188,12 +1188,16 @@ def fetch_metadata(self, commit: bool = True) -> None:
         any_date_col = None
         db_engine_spec = self.database.db_engine_spec
         db_dialect = self.database.get_dialect()
-        dbcols = (
-            db.session.query(TableColumn)
-            .filter(TableColumn.table == self)
-            .filter(or_(TableColumn.column_name == col.name for col in table_.columns))
-        )
-        dbcols = {dbcol.column_name: dbcol for dbcol in dbcols}
+        
+        try:
+            dbcols = (
+                db.session.query(TableColumn)
+                .filter(TableColumn.table == self)
+                .filter(TableColumn.column_name.in_([col.name for col in table.columns]))
+            )
+            dbcols = {dbcol.column_name: dbcol for dbcol in dbcols}
+        except Exception as e:
+            logger.error("Failed to get columns: " + str(e))

Review comment:
       we are trying to catch any exception when we query `table_columns` in sqlite3. If there is no 'try...except...' block, the table seems to be added normally except columns,but there's no any error log. so, i think it's necessary to try to catch any exception here.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] PU-101 commented on a change in pull request #10244: fix: error occurred when fetching large amount of columns

Posted by GitBox <gi...@apache.org>.
PU-101 commented on a change in pull request #10244:
URL: https://github.com/apache/incubator-superset/pull/10244#discussion_r451356940



##########
File path: superset/connectors/sqla/models.py
##########
@@ -1188,12 +1188,16 @@ def fetch_metadata(self, commit: bool = True) -> None:
         any_date_col = None
         db_engine_spec = self.database.db_engine_spec
         db_dialect = self.database.get_dialect()
-        dbcols = (
-            db.session.query(TableColumn)
-            .filter(TableColumn.table == self)
-            .filter(or_(TableColumn.column_name == col.name for col in table_.columns))
-        )
-        dbcols = {dbcol.column_name: dbcol for dbcol in dbcols}
+        
+        try:
+            dbcols = (
+                db.session.query(TableColumn)
+                .filter(TableColumn.table == self)
+                .filter(TableColumn.column_name.in_([col.name for col in table.columns]))
+            )
+            dbcols = {dbcol.column_name: dbcol for dbcol in dbcols}
+        except Exception as e:
+            logger.error("Failed to get columns: " + str(e))

Review comment:
       > What exception are we trying to catch here? In general we should always try to be as specific as possible as to what exception we are expecting to see, as opposed to catching all `Exception`.
   
   we are trying to catch any exception , because i can't identify  possible exception when we query `table_columns` in sqlite3.
   And if there is no `try...except...` block, the table seems to be added normally except columns,but there's no any error log.
   




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org