You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@madlib.apache.org by njayaram2 <gi...@git.apache.org> on 2017/12/11 23:32:39 UTC
[GitHub] madlib pull request #214: Correlation: Fix bug with international characters
GitHub user njayaram2 opened a pull request:
https://github.com/apache/madlib/pull/214
Correlation: Fix bug with international characters
JIRA:MADLIB-1186
If the column name of an independent variable used in
madlib.correlation(...) has quotes in it, then the query fails due
to a regular string concat used for finding the average of a column.
This commit uses add_postfix() to create a new string out of a string
that has special chars instead.
Closes #213
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/njayaram2/madlib bugfix/correlation_international
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/madlib/pull/214.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #214
----
commit d88072988f637789ee6492c8b12e0afaa5e06abf
Author: Nandish Jayaram <nj...@apache.org>
Date: 2017-12-11T22:09:46Z
Correlation: Fix bug with international characters
JIRA:MADLIB-1186
If the column name of an independent variable used in
madlib.correlation(...) has quotes in it, then the query fails due
to a regular string concat used for finding the average of a column.
This commit uses add_postfix() to create a new string out of a string
that has special chars instead.
Closes #213
----
---
[GitHub] madlib pull request #214: Correlation: Fix bug with international characters
Posted by iyerr3 <gi...@git.apache.org>.
Github user iyerr3 commented on a diff in the pull request:
https://github.com/apache/madlib/pull/214#discussion_r156472963
--- Diff: src/ports/postgres/modules/stats/correlation.py_in ---
@@ -179,9 +179,11 @@ def _populate_output_table(schema_madlib, source_table, output_table,
function_name = "Correlation"
agg_str = "{0}.correlation_agg(x, mean)".format(schema_madlib)
- cols = ','.join(["coalesce({0}, avg_{0})".format(col) for col in col_names])
- avgs = ','.join(["avg({0}) AS avg_{0}".format(col) for col in col_names])
- avg_array = ','.join(["avg_{0}".format(col) for col in col_names])
+ cols = ','.join(["coalesce({0}, ".format(col)+add_postfix(col, "_avg")+")"
--- End diff --
Preferred to keep it within the format string i.e.
```
cols = ','.join(["coalesce({0}, {1})".format(col, add_postfix(col, "_avg") for col in col_names])
```
Same applies to the line below.
---
[GitHub] madlib issue #214: Correlation: Fix bug with international characters
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/214
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/307/
---
[GitHub] madlib pull request #214: Correlation: Fix bug with international characters
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/madlib/pull/214
---
[GitHub] madlib pull request #214: Correlation: Fix bug with international characters
Posted by rahiyer <gi...@git.apache.org>.
Github user rahiyer commented on a diff in the pull request:
https://github.com/apache/madlib/pull/214#discussion_r156472675
--- Diff: src/ports/postgres/modules/stats/correlation.py_in ---
@@ -179,9 +179,11 @@ def _populate_output_table(schema_madlib, source_table, output_table,
function_name = "Correlation"
agg_str = "{0}.correlation_agg(x, mean)".format(schema_madlib)
- cols = ','.join(["coalesce({0}, avg_{0})".format(col) for col in col_names])
- avgs = ','.join(["avg({0}) AS avg_{0}".format(col) for col in col_names])
- avg_array = ','.join(["avg_{0}".format(col) for col in col_names])
+ cols = ','.join(["coalesce({0}, ".format(col)+add_postfix(col, "_avg")+")"
--- End diff --
Preferred to keep it within the format string i.e.
```
cols = ','.join(["coalesce({0}, {1})".format(col, add_postfix(col, "_avg") for col in col_names])
```
---
[GitHub] madlib issue #214: Correlation: Fix bug with international characters
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/214
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/304/
---
[GitHub] madlib pull request #214: Correlation: Fix bug with international characters
Posted by njayaram2 <gi...@git.apache.org>.
Github user njayaram2 commented on a diff in the pull request:
https://github.com/apache/madlib/pull/214#discussion_r156476363
--- Diff: src/ports/postgres/modules/stats/correlation.py_in ---
@@ -179,9 +179,11 @@ def _populate_output_table(schema_madlib, source_table, output_table,
function_name = "Correlation"
agg_str = "{0}.correlation_agg(x, mean)".format(schema_madlib)
- cols = ','.join(["coalesce({0}, avg_{0})".format(col) for col in col_names])
- avgs = ','.join(["avg({0}) AS avg_{0}".format(col) for col in col_names])
- avg_array = ','.join(["avg_{0}".format(col) for col in col_names])
+ cols = ','.join(["coalesce({0}, ".format(col)+add_postfix(col, "_avg")+")"
--- End diff --
Makes sense, thank you for the comment @iyerr3.
---