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

added missing keywords for plsql #1501

Closed
wants to merge 6 commits into from
Closed

added missing keywords for plsql #1501

wants to merge 6 commits into from

Conversation

verfriemelt-dot-org
Copy link

these were missing

  • AND
  • FOREACH
  • IN
  • INOUT
  • NEXT
  • NOT
  • OR
  • QUERY
  • VARIADIC

* AND
* FOREACH
* IN
* INOUT
* NEXT
* NOT
* OR
* QUERY
* VARIADIC
@RunDevelopment
Copy link
Member

@verfriemelt-dot-org

Could you please also update the keyword_feature test for plsql?

@verfriemelt-dot-org
Copy link
Author

sure

@verfriemelt-dot-org
Copy link
Author

all done now.

@verfriemelt-dot-org
Copy link
Author

i will replace the spaces with tabs no problem. i implement the missing default operators from pgsql, since there are quite some, which are not present in standard sql, that we use.

so i go ahead and add them all at once, the complete list would be the following.

!
!!
!~
!~*
!~~
!~~*
#
##
#-
#<
#<=
#<>
#=
#>
#>=
#>>
%
&
&&
&<
&<|
&>
*
*<
*<=
*<>
*=
*>
*>=
+
-
->
->>
-|-
/
<
<#>
<->
<<
<<=
<<|
<=
<>
<?>
<@
<^
=
>
>=
>>
>>=
>^
?
?#
?&
?-
?-|
?|
?||
@
@-@
@>
@@
@@@
^
|
|&>
|/
|>>
||
||/
~
~*
~<=~
~<~
~=
~>=~
~>~
~~
~~*

while im at it, i would go ahead and sort out any mixups between operators ( LIKE, AND, OR, XOR ) and keywords.

@verfriemelt-dot-org
Copy link
Author

while fact checking for some keywords i noticed that there are a lot missing, and while i added the all operators i figured, i could add all keywords present in the current pgsql version.

the list is quite long, and im not sure, if this is the right thing to do.

so i could understand if you would reject that PR

@RunDevelopment
Copy link
Member

so I could understand if you would reject that PR

You did nothing wrong. Other languages have huge keyword list too and with a few adjustments, we can make the list a little shorter. ;)

Ok, right now this PR needs a few changes:

  1. We have 757 keywords. This will blow up the file size but with some tricks, we ought to be able to manage it.

    1. Merge them all keywords into one pattern.
      This will eliminate the overhead that comes from /\bword/i. Just make one pattern like before: /\b(?:word1|word2|...)/i. This alone will save us 34% of the minified file size.
    2. Try to group similar patterns together.
      E.g.: ANALY[SZ]E, TYPES?, CHARACTER_SET_(?:NAME|CATALOG|SCHEMA), INTERSECT(?:ION)?
      This will yield especially good results for long keywords. This grouping method is quite tedious to do, so don't overdo it.
      Note: The method used for the last example will only be really efficient if you save >=6 characters.
  2. The same also applies to our 81 operators.
    Same as above: Merge all into one pattern and try to group things together.
    E.g.: <[-#?]?>, #[-#<>=]?

    Be careful when you merge the operators into one pattern! You can trick the otherwise greedy RegEx engine into matching less than it could have. Order matters!
    E.g. /##|###/.exec("###")[0] == "##" but /###|##/.exec("###")[0] == "###"

  3. You don't need to do Prism.languages.plsql['keyword'] = [ ... ]. You can just add 'keyword': [ ... ] anywhere to the Prism.languages.extend('sql', { ... here ... }) object block. (Same for operators)

With these techniques, we should be able to half the current file size to about 5~6kb. Relatively big but manageable.

(And don't forget to update the minified file ^^)

@verfriemelt-dot-org
Copy link
Author

i added this as an array in the current form because i dont know if its feasible with so many keywords, and i woulndt waste my time getting rejected after combining this query.
i will see what i can do, maybe i split this up into pgsql and pgsql extra to avoid having only this big option for using this language.

@mAAdhaTTah
Copy link
Member

@verfriemelt-dot-org This something you're still interested in pursuing?

@RunDevelopment
Copy link
Member

Firstly, please forgive me for my ridiculous demands 3 years ago. I'm sorry.

While reviewing this PR again, I was unable to verify the keywords and operators used in this PR. Since the list of keywords seem to be from an incorrect source or a different language, I will now close this PR.

Sorry @verfriemelt-dot-org!

@verfriemelt-dot-org
Copy link
Author

verfriemelt-dot-org commented Oct 4, 2021

no worries mate ;)

the keywords and operators refered here are comming from postgresql: https://www.postgresql.org/docs/current/sql-keywords-appendix.html and the operators were retrieved directly from a running instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants