-
Notifications
You must be signed in to change notification settings - Fork 473
add porting postgres page #1328
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with some minor nits. Thanks very much, @mjibson!
_data/sidebar_doc.yml
Outdated
|
||
- title: Information Schema | ||
url: /information-schema.html | ||
thirdlevel: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with the rest of the file, would you mind formatting this as follows?
thirdlevel:
- title: Porting Applications
thirdlevelitems:
- title: From PostgreSQL
url: /porting-postgres.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
_data/sidebar_doc.yml
Outdated
- title: Porting Applications | ||
thirdlevelitems: | ||
|
||
- title: Postgres |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be clearer to use the title From PostgreSQL
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
toc: false | ||
--- | ||
|
||
Although CockroachDB supports Postgres syntax and drivers, it does not offer exact compatibility. This page documents the known list of differences between Postgres and CockroachDB for identical input. That is, a SQL statement of the type listed here will behave differently than in Postgres. Porting an existing application to CockroachDB will require changing these expressions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use PostgreSQL
throughout instead of Postgres
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
ERROR: zero raised to a negative power is undefined | ||
``` | ||
|
||
In CockroachDB these expressions will instead return Infinity: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comma after In CockroachDB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
In CockroachDB these expressions will instead return Infinity: | ||
|
||
``` | ||
root@:26257/> select 1e300::float * 1e10::float; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sql prompt, let's use >
. Also, can you capitalize the keywords and break commands and responses into separate code blocks, where the commands have sql
syntax highlighting and the responses have node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
|
||
### Integer division | ||
|
||
Division of integers in Postgres results in an integer. The query: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I'd rewrite this so the code block doesn't interrupt the sentence:
For example, the following query returns
1
, since the1 / 2
is truncated to0
since it is performing integer division:SELECT 1 + 1 / 2
CockroachDB instead provides the
//
operator to perform floor division.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
|
||
In CockroachDB, unary `~` has the same (high) precedence as unary `-`, so the above expression will be parsed as `(~1) + 2`. | ||
|
||
**Porting instructions:** manually add parentheses around expressions that depend on the Postgres behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize Manually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
|
||
Returns `1`, since the `1 / 2` is truncated to `0` since it is performing integer division. In CockroachDB integer division results in a `decimal`. CockroachDB instead provides the `//` operator to perform floor division. | ||
|
||
**Porting instructions:** change `/` to `//` in integer division where the result must be an integer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize Change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
|
||
### Shift argument modulo | ||
|
||
The shift operators in Postgres (`<<`, `>>`) sometimes modulo their second argument to the bit size of the underlying type. For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, revise so the sample doesn't interrupt the sentence:
For example, the following query results in a
1
because theint
type is 32 bits, and32 % 32
is0
, so this is the equivalent of1 << 0
:SELECT 1::int << 32
In CockroachDB, no such modulo is performed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
porting-postgres.md
Outdated
|
||
Results in a `1` because the `int` type is 32 bits, and `32 % 32` is `0`, so this is the equivalent of `1 << 0`. In CockroachDB, no such modulo is performed. | ||
|
||
**Porting instructions:** manually add a modulo to the second argument. Also note that CockroachDB's int type is always 64 bits. For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize Manually
. Also, let's capitalize INT
and link to our docs:
[`INT`](int.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with a few final comments
porting-postgres.md
Outdated
SELECT ~1 + 2 | ||
``` | ||
|
||
Is parsed as `~ (1 + 2)` because `~` has a lower precedence than `+`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this line anymore.
porting-postgres.md
Outdated
|
||
### Integer division | ||
|
||
Division of integers in PostgreSQL results in an integer. For example, the following query returns `1`, since the `1 / 2` is truncated to `0` since it is performing integer division: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd actually remove since it is performing integer division
. Feels redundant and makes the sentence a bit convoluted.
I'd recommend mentioning that this guide primarily focuses on expressions and does not demonstrate how to port applications that use unsupported SQL features (e.g. arrays, enum). |
Good point, @sploiselle. We may want to expand this page to cover a lot more, but for now, totally agree that it's best to be clear about the focus. |
Review status: 0 of 2 files reviewed at latest revision, 15 unresolved discussions. porting-postgres.md, line 9 at r3 (raw file):
After this line, recommend adding something like: {{site.data.alerts.callout_info}}This document currently only covers how to rewrite SQL expressions. It does not discuss strategies for porting application that use SQL features CockroachDB does not currently support, such as arrays or the Comments from Reviewable |
I've made those changes. RFAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with one final nit.
porting-postgres.md
Outdated
|
||
Note that some of these differences only apply to rare inputs, and so no change will be needed, even if the listed feature is being used. In these cases, it is safe to ignore the porting instructions. | ||
|
||
{{site.data.alerts.callout_info}}This document currently only covers how to rewrite SQL expressions. It does not discuss strategies for porting application that use [SQL features CockroachDB does not currently support](/sql-feature-support.html), such as arrays or the `ENUM` type.{{site.data.alerts.end}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
porting applications
- needs to be plural
…ckroachdb#1328) * Optimize access of last element in traversals * Remove no longer existing patch-update-statements AQL optimizer rule * Account for optimizer rules that are applied a 3rd time (-3) suffix --------- Co-authored-by: Paula Mihu <97217318+nerpaula@users.noreply.github.com>
Fixes #1327
This change is