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

scalar-expressions-and-types.mdの更新 #34

Merged
merged 4 commits into from
Apr 4, 2024

Conversation

kuron99
Copy link
Contributor

@kuron99 kuron99 commented Apr 2, 2024

  • 代入変換でto approx. numberは緩めにするという方針を受けて、精度は落ちない扱いとする
  • 文字列間の変換で national パラメータに言及があった部分を削除
  • 文字列の長さを減らす際にUTF-8文字単位で扱うことを明記
  • キャスト変換のマトリクスで文字列間で変換エラーが起きるケースを削除

@kuron99 kuron99 requested a review from ashigeru as a code owner April 2, 2024 00:31
@kuron99
Copy link
Contributor Author

kuron99 commented Apr 3, 2024

@ashigeru 文字列の national を削除しました。レビューお願いします。(タイムゾーン関連の修正は見送りました)

あと一点質問ですが、キャスト変換の表で下記が D になっています。精度を失うことはあっても変換エラーになることはないと思うので、これは v が正しいのではないでしょうか?

  • character varying から character
  • character から character varying

Copy link
Contributor

@ashigeru ashigeru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

基本的にOKですが、コメントした箇所について意見ください。

`v'` の長さが `T` の `length` よりも大きい場合、 `v'` を `T` の `length` まで末尾からオクテットを取り除いたものが式の評価結果となる。
~ 文字列を表す値 `v` に対し、 `v` の長さが `T` の `length` と等しい場合、 `v` が式の評価結果となる。
`v` の長さが `T` の `length` よりも小さい場合、 `v` を `T` の `length` まで末尾にパディング文字を加えたものが式の評価結果となる。
`v` の長さが `T` の `length` よりも大きい場合、 `v` を `T` の `length` まで末尾からオクテットを取り除いたものが式の評価結果となる。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ちょっと思ったのですが、この仕様だと UTF-8 が壊れる気がするけどどうですかね。
char にバイト列入れていた場合とぶつかりそうだから何とも言えないところではあるんですが。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かにそうですね。validなUTF-8文字の境界で区切るようにすべきな気がします。char/varcharはUTF-8専用で、バイナリ列はoctet/octet varyingを使うという方針がいいように思います。下記のような感じでしょうか。

v の長さが Tlength よりも大きい場合、 Tlength 以下になるまで末尾からUTF-8文字を取り除いたものが式の評価結果となる。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

で、削りすぎた箇所にパディングを入れるかどうかという問題が

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

変換後の型がcharacterの場合、characterはUTF-8文字列であって長さがちょうど length バイトであるもの、ということなので、削った結果が length バイトに足りてなければパディング一択ではないでしょうか。
character variantの場合はパディングする理由がないので問題にならないですね。

`v'` の長さが `T` の `length` よりも大きい場合、 `v'` を `T` の `length` まで末尾からオクテットを取り除いたものが式の評価結果となる。
~ 文字列を表す値 `v` に対し、 `v` の長さが `T` の `length` と等しい場合、 `v` が式の評価結果となる。
`v` の長さが `T` の `length` よりも小さい場合、 `v` を `T` の `length` まで末尾にパディング文字を加えたものが式の評価結果となる。
`v` の長さが `T` の `length` よりも大きい場合、 `v` を `T` の `length` まで末尾からオクテットを取り除いたものが式の評価結果となる。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらも同様。

@ashigeru
Copy link
Contributor

ashigeru commented Apr 4, 2024

@ashigeru 文字列の national を削除しました。レビューお願いします。(タイムゾーン関連の修正は見送りました)

あと一点質問ですが、キャスト変換の表で下記が D になっています。精度を失うことはあっても変換エラーになることはないと思うので、これは v が正しいのではないでしょうか?

  • character varying から character
  • character から character varying

キャストの内容を見たところ、エラーになる余地がないのでご指摘の通りかと。

@kuron99
Copy link
Contributor Author

kuron99 commented Apr 4, 2024

@ashigeru 上記コメントを反映しました。再度確認お願いします。

Copy link
Contributor

@ashigeru ashigeru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kuron99 kuron99 merged commit 240d970 into master Apr 4, 2024
6 checks passed
@kuron99 kuron99 deleted the docs_lost_prec_converted_to_float branch April 4, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants