Initial stuff #1
Conversation
garyb
left a comment
garyb
left a comment
There was a problem hiding this comment.
A couple of comments, I'll stop for now and then resume again once we've decided whether to do the EJson stuff, etc.
src/SqlSquare/AST.purs
Outdated
|
|
||
| import Matryoshka (class Recursive, Algebra, cata, transParaT, project) | ||
|
|
||
| import Debug.Trace as DT |
| , orderBy ∷ Maybe (OrderBy a) | ||
| } | ||
| | Select (SelectR a) | ||
| | Parens a |
There was a problem hiding this comment.
Do you think we could use EJson rather than having constructors for all the literals in here? I don't think there are any EJson literals that don't have a corresponding SQL^2 representation. SQL^2 will need an additional set literal though.
If there's some problem with that I'm not thinking of, I have another related suggestion - although this is kinda a personal-preference thing - there might be some value in separating literals out into their own ADT, and just having a Literal (LiteralF a) constructor in SqlF. It simplified some stuff when I applied it in PS anyway.
There was a problem hiding this comment.
Yes, using EJson is a good idea. Technically you could use Coproduct SqlF EJsonF where SqlF is the part of SQL2 that is not EJson.
Remember in SQL2 you can do things like this:
SELECT {d.dynamic_field : [d.another_one]} FROM `/foo/` AS d
i.e. "literals" can recursively contain arbitrary SQL2 / EJSON.
| ArrayLiteral lst → | ||
| ArrayLiteral $ map f lst | ||
| Parens t → | ||
| Parens $ f t |
There was a problem hiding this comment.
Nope, there is Tuple a a
src/SqlSquare/JoinType.purs
Outdated
| LeftJoin → "left join" | ||
| RightJoin → "right join" | ||
| FullJoin → "full join" | ||
| InnerJoin → "inner join" |
There was a problem hiding this comment.
This is a general comment, but here seems like a good a place as any... should we render the SQL-syntax-stuff as all uppercase perhaps? That seems more conventional to me; and I tend to write it that way when doing so by hand as it makes it easier to spot the parts that are identifiers, etc.
src/SqlSquare/Lenses.purs
Outdated
| import SqlSquare.Utils (type (×), (∘), (⋙)) | ||
|
|
||
| _Newtype ∷ ∀ n t. Newtype n t ⇒ Iso' n t | ||
| _Newtype = iso unwrap wrap |
There was a problem hiding this comment.
There's already an iso for this: https://pursuit.purescript.org/packages/purescript-profunctor-lenses/2.6.0/docs/Data.Lens.Iso.Newtype#v:_Newtype
src/SqlSquare/Constructors.purs
Outdated
| import SqlSquare.AST (SqlF(..), Relation, GroupBy(..), OrderBy, BinaryOperator, UnaryOperator, (∘), SelectR, Case(..), Projection(..)) | ||
|
|
||
| vari ∷ ∀ t. Corecursive t SqlF ⇒ String → t | ||
| vari ∷ ∀ t. Corecursive t (SqlF EJsonF) ⇒ String → t |
There was a problem hiding this comment.
I think we can leave EJsonF polymorphic for a bunch of these...
|
|
Ah! Yep, added |
@garyb is this good to go? |
garyb
left a comment
garyb
left a comment
There was a problem hiding this comment.
It's going to be great to be able to replace our string munging with this!
| foldr f a (OrderBy xs) = F.foldr (flip (F.foldr f)) a xs | ||
| instance traversableOrderBy ∷ T.Traversable OrderBy where | ||
| traverse f (OrderBy xs) = map OrderBy $ T.traverse (T.traverse f) xs | ||
| sequence = T.sequenceDefault |
There was a problem hiding this comment.
I think derive newtype instance should work for Foldable / Traversable here?
src/SqlSquare/Lenses.purs
Outdated
|
|
||
| _Splice | ||
| ∷ ∀ t | ||
| . (Recursive t (S.SqlF EJ.EJsonF), Corecursive t (S.SqlF EJ.EJsonF)) |
There was a problem hiding this comment.
This is a pretty minor thing, but we could make the lenses that aren't literal-specific polymorphic in SqlF l too perhaps. I wouldn't hold up this PR for that though!
|
I've made lenses more polymorphic, but |
|
Oh well! Just thought it was worth a try. |
3 participants
Printer, search interpreter and
JArrayexample@garyb Could you please take a look?