Add codec for LTREE type. #544
Conversation
|
Cool! I will have a look. |
Codecov Report
@@ Coverage Diff @@
## main #544 +/- ##
==========================================
+ Coverage 84.75% 84.81% +0.06%
==========================================
Files 124 126 +2
Lines 1692 1719 +27
Branches 114 117 +3
==========================================
+ Hits 1434 1458 +24
- Misses 258 261 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| object LTree { | ||
| val Empty = LTree(Nil) | ||
|
|
||
| def unsafe(labels: String*): LTree = LTree(labels.toList) |
There was a problem hiding this comment.
Let's leave this out. The user can deal with the case where they're sure it's valid. We could also add a ltree"..." literal handler at some point.
There was a problem hiding this comment.
I can try to add a literal. I've never built one before but seems like the Literally library might be a good way to go? Or would you prefer to avoid adding the dependency? At a quick glance it looks like it would need to be done independently for scala 2/3.
There was a problem hiding this comment.
Yes, Literally is the way to go. We can save it for a followup.
|
|
||
| def unsafe(labels: String*): LTree = LTree(labels.toList) | ||
|
|
||
| def fromString(s: String): Either[String, LTree] = { |
There was a problem hiding this comment.
It seems like we could do all of this with a single regex.
There was a problem hiding this comment.
You're probably right. My regex-foo is limited but I can try to tackle it when I get a little more free time.
| val MaxTreeLength = 65535 | ||
|
|
||
| private val Separator = '.' | ||
| private val ValidLabelRegex = s"""^[A-Za-z0-9_]{1,$MaxLabelLength}$$""".r |
There was a problem hiding this comment.
This is actually more restrictive than the implementation, which apparently depends on the character encoding, which Skunk requires to be UTF-8. So foo.βar.baz is a legal lpath. But unless we look at the source it's unlikely we'll be able to figure out exactly what it's looking for.
There was a problem hiding this comment.
I adjusted the label regex such that it seems to pass your example but I'm sure that doesn't necessarily meet the spec.
There was a problem hiding this comment.
Yeah the spec is probably "whatever the implementation does", which we might want to look at. It would be nice to follow the same rules on the Skunk end.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Is this PR ready to merge? I'm using ltree in a project |
Adds a codec for the postgresql ltree type.