Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MapD] Resolves Joining with different column names #1647

Closed
wants to merge 12 commits into from

Conversation

Projects
2 participants
@xmnlab
Copy link
Collaborator

commented Oct 2, 2018

Fixes #1619

@@ -596,6 +596,7 @@ class ByteLength(ops.StringLength):
# GENERAL
_general_ops = {
ops.Literal: literal,
ops.NullLiteral: lambda *args: 'CAST(NULL AS FLOAT)',

This comment has been minimized.

Copy link
@cpcloud

cpcloud Oct 4, 2018

Member

I don't think this is the correct fix for this. We need to cast to the correct type wherever NULL is being used. In the case of #1620, it's in the code that the bucket method is generating.

This comment has been minimized.

Copy link
@xmnlab

xmnlab Oct 4, 2018

Author Collaborator

I see ... could you provide more example or explanation about that? I am not sure how to get the type if I just use ibis.literal(None)

@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 7, 2018

@xmnlab I will fix the null type issue in a different PR. Basically, we need to cast the null in the case statement to the results type so that there isn't an untyped null hanging around.

@xmnlab

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 8, 2018

@cpcloud thank you so much! If you can ping me in the PR, please ... I really want to learn how to do this :)
I will remove the literal null changes in this PR.

@codecov

This comment has been minimized.

Copy link

commented Oct 12, 2018

Codecov Report

Merging #1647 into master will decrease coverage by 2.66%.
The diff coverage is 84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1647      +/-   ##
==========================================
- Coverage   90.01%   87.35%   -2.67%     
==========================================
  Files         185      185              
  Lines       27283    27223      -60     
  Branches     2329     2338       +9     
==========================================
- Hits        24559    23780     -779     
- Misses       2323     3036     +713     
- Partials      401      407       +6
Impacted Files Coverage Δ
ibis/mapd/tests/test_operations.py 95.45% <100%> (-0.98%) ⬇️
ibis/mapd/tests/conftest.py 61.7% <100%> (+3.56%) ⬆️
ibis/mapd/compiler.py 81.88% <71.42%> (+2.21%) ⬆️
ibis/bigquery/tests/test_client.py 25.87% <0%> (-73.55%) ⬇️
ibis/bigquery/tests/test_compiler.py 39.89% <0%> (-60.11%) ⬇️
ibis/bigquery/udf/tests/test_udf_execute.py 28% <0%> (-54.18%) ⬇️
ibis/bigquery/client.py 42.91% <0%> (-52.37%) ⬇️
ibis/bigquery/compiler.py 58.82% <0%> (-38.61%) ⬇️
ibis/bigquery/tests/conftest.py 70.27% <0%> (-16.22%) ⬇️
ibis/bigquery/udf/api.py 80.48% <0%> (-14.64%) ⬇️
... and 34 more

@xmnlab xmnlab changed the title [MapD] Resolves Literal Null and Joining on different column names [MapD] Resolves Joining with different column names Oct 12, 2018

xmnlab added some commits Oct 15, 2018

@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 20, 2018

@xmnlab Can you add a test for this?

@cpcloud cpcloud added mapd bug labels Oct 20, 2018

@cpcloud cpcloud added this to the Next Release milestone Oct 20, 2018

@cpcloud cpcloud added this to In progress in MapD via automation Oct 20, 2018

@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 20, 2018

@xmnlab Can you rebase and push all of your PRs?

@xmnlab

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 23, 2018

@cpcloud sure! I am working on that! thanks!

@xmnlab

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 23, 2018

it raised a error for maiadb installation on windows+py36

ERROR: The remote file either doesn't exist, is unauthorized, or is forbidden for url 'https://downloads.mariadb.org/f/mariadb-10.2.14/winx64-packages/mariadb-10.2.14-winx64.msi'. Exception calling "GetResponse" with "0" argument(s): "The remote server returned an error: (403) Forbidden."
2018-10-23T02:19:58.6711299Z The install of mariadb was NOT successful.
@xmnlab

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 25, 2018

azure pipelines is still raising error on windows_test py35 for mariadb:

2018-10-25T16:51:17.8292526Z Downloading mariadb 64 bit
2018-10-25T16:51:17.8293327Z   from 'https://downloads.mariadb.org/f/mariadb-10.2.14/winx64-packages/mariadb-10.2.14-winx64.msi'
2018-10-25T16:51:20.3065302Z ERROR: The remote file either doesn't exist, is unauthorized, or is forbidden for url 'https://downloads.mariadb.org/f/mariadb-10.2.14/winx64-packages/mariadb-10.2.14-winx64.msi'. Exception calling "GetResponse" with "0" argument(s): "The remote server returned an error: (403) Forbidden."
2018-10-25T16:51:20.6532187Z The install of mariadb was NOT successful.
2018-10-25T16:51:20.6551619Z Error while running 'C:\ProgramData\chocolatey\lib\mariadb\tools\chocolateyInstall.ps1'.
2018-10-25T16:51:20.6551954Z  See log for details.
2018-10-25T16:51:22.7495596Z 
2018-10-25T16:51:22.7496068Z Chocolatey installed 0/1 packages. 1 packages failed.
2018-10-25T16:51:22.7496140Z  See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).
2018-10-25T16:51:22.7505864Z 
2018-10-25T16:51:22.7511324Z Failures
2018-10-25T16:51:22.7518311Z  - mariadb (exited 404) - Error while running 'C:\ProgramData\chocolatey\lib\mariadb\tools\chocolateyInstall.ps1'.
2018-10-25T16:51:22.7518456Z  See log for details.
2018-10-25T16:51:23.5270254Z ##[error]Cmd.exe exited with code '404'.
@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

@xmnlab Can you rebase your PRs again? The failing builds should be fixed as of #1667 (among other things a new version of flake8 caused some failed linting). I will review your PRs in the meantime.

@xmnlab

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 27, 2018

thanks @cpcloud! rebased!

@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 28, 2018

@xmnlab can your resolve the merge conflicts? i will merge after that.

MapD automation moved this from In progress to Reviewer approved Oct 28, 2018

@xmnlab

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 29, 2018

rebased! thanks @cpcloud !

@cpcloud cpcloud closed this in afe35e1 Oct 30, 2018

MapD automation moved this from Reviewer approved to Done Oct 30, 2018

@xmnlab xmnlab deleted the Quansight:add_literal_null branch Mar 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.