You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@madlib.apache.org by GitBox <gi...@apache.org> on 2020/07/24 17:13:52 UTC
[GitHub] [madlib] orhankislal opened a new pull request #508: Various: Do not drop output tables
orhankislal opened a new pull request #508:
URL: https://github.com/apache/madlib/pull/508
JIRA: MADLIB-1442
We noticed that some MADlib modules drop output tables before
recreating them. This is not a safe assumption since the user might
put a table name that they actually need by mistake. This commit
adds checks for existing output tables and removes the drop statements.
<!--
Thanks for sending a pull request! Here are some tips for you:
1. Refer to this link for contribution guidelines https://cwiki.apache.org/confluence/display/MADLIB/Contribution+Guidelines
2. Please Provide the Module Name, a JIRA Number and a short description about your changes.
-->
- [ ] Add the module name, JIRA# to PR/commit and description.
- [ ] Add tests for the change.
----------------------------------------------------------------
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
[GitHub] [madlib] orhankislal commented on a change in pull request #508: Various: Do not drop output tables
Posted by GitBox <gi...@apache.org>.
orhankislal commented on a change in pull request #508:
URL: https://github.com/apache/madlib/pull/508#discussion_r465922367
##########
File path: src/ports/postgres/modules/glm/glm.py_in
##########
@@ -250,7 +250,6 @@ def __glm_compute(schema_madlib, tbl_source, tbl_output, col_dep_var, col_ind_va
plpy.execute(
"""
- DROP TABLE IF EXISTS {tbl_output};
Review comment:
Line 102 already does the `output_tbl_valid` function call.
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 commented on pull request #508: Various: Do not drop output tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 commented on pull request #508:
URL: https://github.com/apache/madlib/pull/508#issuecomment-670027767
```
madlib=# SELECT * FROM madlib.summary( 'iris', -- Source table
madlib(# 'pg_temp.iris_summary' -- Output table
madlib(# );
output_table | num_col_summarized | duration
----------------------+--------------------+-------------------
pg_temp.iris_summary | 6 | 0.192517042160034
(1 row)
```
LGTM
----------------------------------------------------------------
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
[GitHub] [madlib] fmcquillan99 edited a comment on pull request #508: Various: Do not drop output tables
Posted by GitBox <gi...@apache.org>.
fmcquillan99 edited a comment on pull request #508:
URL: https://github.com/apache/madlib/pull/508#issuecomment-670027767
```
madlib=# SELECT * FROM madlib.summary( 'iris', -- Source table
madlib(# 'pg_temp.iris_summary' -- Output table
madlib(# );
output_table | num_col_summarized | duration
----------------------+--------------------+-------------------
pg_temp.iris_summary | 6 | 0.192517042160034
(1 row)
```
tested on
```
PostgreSQL 8.3.23 (Greenplum Database 5.18.0 build commit:6aec9959d367d46c6b4391eb9ffc82c735d20102) on x86_64-pc-linux-gnu, compiled by GCC gcc (GCC) 6.2.0, 64-bit compiled
on Apr 3 2019 14:45:51
```
LGTM
----------------------------------------------------------------
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
[GitHub] [madlib] khannaekta commented on a change in pull request #508: Various: Do not drop output tables
Posted by GitBox <gi...@apache.org>.
khannaekta commented on a change in pull request #508:
URL: https://github.com/apache/madlib/pull/508#discussion_r464727148
##########
File path: src/ports/postgres/modules/glm/glm.py_in
##########
@@ -250,7 +250,6 @@ def __glm_compute(schema_madlib, tbl_source, tbl_output, col_dep_var, col_ind_va
plpy.execute(
"""
- DROP TABLE IF EXISTS {tbl_output};
Review comment:
We should probably call `output_tbl_valid(tbl_output)`
----------------------------------------------------------------
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
[GitHub] [madlib] orhankislal merged pull request #508: Various: Do not drop output tables
Posted by GitBox <gi...@apache.org>.
orhankislal merged pull request #508:
URL: https://github.com/apache/madlib/pull/508
----------------------------------------------------------------
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