vscode-ibmi
vscode-ibmi copied to clipboard
Component API
Changes
As we increase our dependence on our own SQL tools, I think it was time we added an API to easily add and maintain different services.
As well as that, this PR does another of other things internally:
- Renamed
runtimeCcsid
tojobCcsid
and also addedqccsid
on the IBMi class. - Moved
IBMiContent
out ofInstance
and into theIBMi
class. - Checks for the status of the component every time we connect. This data is not cached.
- the
IBMi#runSQL
method will, depending on the QCCSID, fetch the data from a CSV using one of two of our new components. This will solve a number of CCSID issues when working with EBCDIC in tables as we read only in UTF8.- We fallback to
CPYTOIMPF
if the statement is 'simple' as it is more performant thanSQL_TO_CSV
- We fallback to
CPYTOIMPF
ifSQL_TO_CSV
fails to install
- We fallback to
-
IBMiContent#getTable
now only usesCPYTOIMPF
and does not resolve to SQL when possible. We still use this API to fetch a specific member during the complication process (EVFEVENT file members) - Added new tests for multiple encodings. More are welcome.
Nice to haves
Below are some CCSID related issues which would go along well with this PR:
- #1993 for when QCCSID is valid
- #1992 being implemented to solve some CCSID issues when fetching the object list
To do
- [x] Install components if not installed
- [x] Check the state of a component (installed or not)
- [x] Improve version control of individual components
- [x] Added
SQL_TO_CSV
component - [x] Added
IFS_WRITE
component - [x] Inner component dependencies? (
SQL_TO_CSV
depends onIFS_WRITE
) - [x] Fix handling of null values in
SQL_TO_CSV
- [ ] Write components to a new dedicated library if available
- [x] Test cases for
runSQL
API usingSQL_TO_CSV
- [x] Encoding tests (steal from #1983) to verify
SQL_TO_CSV
- [ ] Improve type system of
ComponentManager
- [ ] Improve performance of
getMemberList
- [ ] Convert
IFS_WRITE
to have aclob
parameter overvarchar
- [x] #1968 merged
How to test this PR
Examples:
- Run the test cases
Checklist
- [x] have tested my change
- [ ] have created one or more test cases
- [ ] updated relevant documentation
- [ ] Remove any/all
console.log
s I added - [ ] have added myself to the contributors' list in CONTRIBUTING.md
@worksofliam @sebjulliand I think it would be great if the extension would
- check the component versions installed on the server on first connect after extension update and install any component not there or not up to date
- use a versioning method that also covers CL objects - CL programs don't have long comment like SQL objects, and we may introduce non-SQL components later on (we had that for command definitions before it was moved to the CL extension). We could use the object text to have the version information, e.g. 'v1 Code for IBM i: Get something...'.
@chrjorgensen Per your second point: each component has their own getInstalledVersion
method which returns a number. Each component can have their own version checker. So for CL, we might use SQL to get the object text, whereas for SQL objects we might use the comments.
@codefori/core Any chance I can get a midpoint review on this API changes so far?
Here are some of the failing tests due to some issues in SQL_TO_CSV
that I am going to need some help with:
This one looks like it should be passing?
@sebjulliand Thanks for your review and commits! I take it you are okay with my implementation of the getComponent
API?
@chrjorgensen Any chance I can get your help correcting / adding the GRANT
to each SQL component here after the create
statements?
@worksofliam Sure. Do we agree that all users should be able to replace the SQL components?
@worksofliam I added GRTOBJAUT
to all SQL components instead of GRANT
... since GRANT
does not give *OBJMGT
required when replacing a function / procedure.
https://www.ibm.com/docs/en/i/7.5?topic=statements-create-function-sql-table
We still have two failing tests which, until I have fixed, will hold this PR up:
This one less important:
CALL ILEDITOR.SQL_TO_CSV('With MEMBERS As ( SELECT rtrim(cast(a.system_table_schema as char(10) )) as LIBRARY, b.avgrowsize as RECORD_LENGTH, a.iasp_number as ASP, rtrim(cast(a.system_table_name as char(10) )) AS SOURCE_FILE, rtrim(cast(b.system_table_member as char(10) )) as NAME, coalesce(rtrim(cast(b.source_type as varchar(10) )), '''') as TYPE, coalesce(rtrim(varchar(b.partition_text)), '''') as TEXT, b.NUMBER_ROWS as LINES, extract(epoch from (b.CREATE_TIMESTAMP))*1000 as CREATED, extract(epoch from (b.LAST_SOURCE_UPDATE_TIMESTAMP))*1000 as CHANGED FROM qsys2.systables AS a JOIN qsys2.syspartitionstat AS b ON b.table_schema = a.table_schema AND b.table_name = a.table_name ) Select * From MEMBERS Where LIBRARY = ''QSYSINC'' And SOURCE_FILE = ''QRPGLESRC'' Order By NAME DESC', '/tmp/vscodetemp-O_jj1aTKjM')
--DB2>
-- ?>
--
-- **** CLI ERROR *****
-- SQLSTATE: 22001
--NATIVE ERROR CODE: -302
--Conversion error on variable or parameter SQLP_L2.FILE_CONTENT.
The cause of the conversion error is when the the call the IFS_WRITE
happens.
SQL_TO_CSV
has a variable named file_content
, which is a clob type, but IFS_WRITE
has a varchar(32k) type. It works when the clob data is small enough for the varchar, but after 32k bytes, then it causes this error.
Niels provided me with an updated IFS_WRITE
that supports the clob parameter. I need to update the version used here to use that instead.
https://gist.github.com/NielsLiisberg/cd2350aee85f5b2e967993faf7ea7595
@sebjulliand @chrjorgensen
This PR is now open again and ready to be tested. Some questions:
-
SQL_TO_CSV
andIFS_WRITE
were added to this PR, but it is not using them. I hate to say it, butCPYTOSTMF
is significantly faster thanSQL_TO_CSV
, as proven by the test suite (specifically the 'getMemberList (advanced filtering)' test). Do you think we should just remove those component?
@worksofliam Didn't we start using pure SQL to get the line numbers and line dates from members, which CPYTOSTMF does not provide?
If people using these are punished performance-wize, so be it. Could be a motivator for moving to streamfiles and git! 😉
@chrjorgensen
which CPYTOSTMF does not provide?
Right, but we only use CPYTOIMPF on temporary objects in QTEMP
!
@sebjulliand @chrjorgensen
This PR is now open again and ready to be tested. Some questions:
SQL_TO_CSV
andIFS_WRITE
were added to this PR, but it is not using them. I hate to say it, butCPYTOSTMF
is significantly faster thanSQL_TO_CSV
, as proven by the test suite (specifically the 'getMemberList (advanced filtering)' test). Do you think we should just remove those component?
I had a bit of chat with Liam, the potential use cases are all best handled by the DB2 for i extension. It probably increases the attack surface and we can get it back easily enough, so I recommend deleting them
@sebjulliand I have removed SQL_TO_CSV
and IFS_WRITE
from this PR seeing as they were completely disabled anyway. Tests are still passing as expected (other than the one about tabs!)
(Whoops, I ran the tests without a workspace open!!)