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

Improve handling of bytes data #7062

Merged
merged 1 commit into from Mar 20, 2019
Merged

Improve handling of bytes data #7062

merged 1 commit into from Mar 20, 2019

Conversation

villebro
Copy link
Member

@villebro villebro commented Mar 19, 2019

This is a continuation of #6987 which caused a regression when selecting non-utf-8 bytes data on Postgres. Also improves and harmonizes rendering of utf-8 compliant bytes data when possible.

Before

  • Postgres utf-8 bytes data: 🐍 shows up as 🐍
  • Postgres non-utf-8 bytes data: åäöÅÄÖ encoded as latin-1 throws exception
  • Non-Postgres utf-8 bytes data: 🐍 shows up as b'\\xf0\\x9f\\x90\\x8d'
  • Non-Postgres non-utf-8 bytes data: åäöÅÄÖ encoded as latin-1 shows up as b'\\xe5\\xe4\\xf6\\xc5\\xc4\\xd6'

After

  • Postgres utf-8 bytes data: 🐍 shows up as 🐍
  • Postgres non-utf-8 bytes data: åäöÅÄÖ encoded as latin-1 shows up as [bytes]
  • Non-Postgres utf-8 bytes data: 🐍 shows up as 🐍
  • Non-Postgres non-utf-8 bytes data: åäöÅÄÖ encoded as latin-1 shows up as [bytes]

Screenshot 2019-03-19 at 19 07 13

Ping @mistercrunch @john-bodley @mmuru

@codecov-io
Copy link

Codecov Report

Merging #7062 into master will not change coverage.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7062   +/-   ##
=======================================
  Coverage   64.46%   64.46%           
=======================================
  Files         421      421           
  Lines       20537    20537           
  Branches     2247     2247           
=======================================
  Hits        13240    13240           
  Misses       7170     7170           
  Partials      127      127
Impacted Files Coverage Δ
superset/utils/core.py 88.22% <33.33%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7ffdd6...02ae43c. Read the comment docs.

@kristw kristw added the #refine label Mar 19, 2019
@mmuru
Copy link
Contributor

mmuru commented Mar 19, 2019

@villebro: I verified the changes and it worked in preview and run query modes. 👍

@mistercrunch mistercrunch merged commit c1c8e50 into apache:master Mar 20, 2019
@kristw kristw added enhancement:request Enhancement request submitted by anyone from the community and removed #refine labels Mar 20, 2019
@villebro villebro deleted the bytes_mk2 branch March 20, 2019 08:14
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels enhancement:request Enhancement request submitted by anyone from the community 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants