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

sql: INET columns are not formatted during pretty-printing #23850

Closed
nvanbenschoten opened this issue Mar 14, 2018 · 4 comments
Closed

sql: INET columns are not formatted during pretty-printing #23850

nvanbenschoten opened this issue Mar 14, 2018 · 4 comments
Assignees
Labels
A-sql-execution Relating to SQL execution. C-investigation Further steps needed to qualify. C-label will change. E-easy Easy issue to tackle, requires little or no CockroachDB experience S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Milestone

Comments

@nvanbenschoten
Copy link
Member

The following is the result of an EXPLAIN statement. It demonstrates that INET values are not properly formatted when pretty printing.

root@:26257/inettest> explain select * from users where addr = '2001:db8::68/0':::INET;
+------+-------+--------------------------------------------------------------------------+
| Tree | Field |                               Description                                |
+------+-------+--------------------------------------------------------------------------+
| scan |       |                                                                          |
|      | table | users@users_addr_idx                                                     |
|      | spans | /"\x01\x00                                                               |
|      |       | \x01\r\xb8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00h"-/"\x01\x00      |
|      |       | \x01\r\xb8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00h"/PrefixEnd       |
+------+-------+--------------------------------------------------------------------------+
(3 rows)
@nvanbenschoten nvanbenschoten added S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. O-qa labels Mar 14, 2018
@nvanbenschoten nvanbenschoten added this to the 2.1 milestone Mar 14, 2018
@knz knz removed the O-qa label Apr 24, 2018
@knz knz added the C-investigation Further steps needed to qualify. C-label will change. label Apr 24, 2018
@knz knz added the A-sql-execution Relating to SQL execution. label May 15, 2018
@knz knz modified the milestones: 2.1, 2.2 Oct 5, 2018
@nvanbenschoten
Copy link
Member Author

@jordanlewis this might be a good starter project for a new engineer joining the SQL team.

@jordanlewis jordanlewis added the E-easy Easy issue to tackle, requires little or no CockroachDB experience label Apr 24, 2019
@solongordon solongordon modified the milestones: 19.1, 19.2 May 30, 2019
@rohany
Copy link
Contributor

rohany commented Jun 4, 2019

@nvanbenschoten I found the root of the issue, but is unclear how to fix the problem in a backwards compatible way. In EncodeTableKey in src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/column_type_encoding.go, IPAddr values are encoded as byte streams, but they are given bytes tag, rather than an IPAddr tag for decoding. This results in the value not being decoded as an IPAddr when it is pretty printed. Fixing this requires changing the encoding the IPAddr objects which would differ with encodings that exist right now.

@nvanbenschoten
Copy link
Member Author

So this is only an issue with IPAddr datums that are encoded in keys, not values, right? Do we have similar issues with other datum types? My understanding of this area of code is pretty out-of-date. Perhaps @solongordon has more information on whether this is acceptable behavior, and if not, what we should do about it.

@nvanbenschoten
Copy link
Member Author

I'm not sure when this was addressed, but it seems to be fixed now:

demo@127.0.0.1:26257/movr> create table t (addr inet primary key);
CREATE TABLE

Time: 2ms total (execution 2ms / network 0ms)

demo@127.0.0.1:26257/movr> explain select * from t where addr = '2001:db8::68/0':::INET;
                        info
----------------------------------------------------
  distribution: local
  vectorized: true

  • scan
    missing stats
    table: t@primary
    spans: [/'2001:db8::68/0' - /'2001:db8::68/0']
(7 rows)

Time: 2ms total (execution 2ms / network 0ms)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. C-investigation Further steps needed to qualify. C-label will change. E-easy Easy issue to tackle, requires little or no CockroachDB experience S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

No branches or pull requests

5 participants