Bug 146281 - [Feature Request] BaseDocumenter extension in Firebird not working
Summary: [Feature Request] BaseDocumenter extension in Firebird not working
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Extensions (show other bugs)
Version:
(earliest affected)
7.2.3.2 release
Hardware: All All
: medium enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0 target:7.3.1
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-17 17:35 UTC by malt25
Modified: 2022-02-10 08:21 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description malt25 2021-12-17 17:35:31 UTC
Description:
The helpful extension BaseDocumenter, which gives a detailed overview of an database, does not function in Firebird, but in HSQLDB. Perhaps future LO versions will allow it to run in Firebird too.

Version: 7.2.4.1 (x64) / LibreOffice Community
Build ID: 27d75539669ac387bb498e35313b970b7fe9c4f9
CPU threads: 6; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: de-DE
Calc: CL

Steps to Reproduce:
1.Try to run the BaseDocumenter extension
2.
3.

Actual Results:
Does not work

Expected Results:
BaseDocumenter extension runs


Reproducible: Always


User Profile Reset: Yes



Additional Info:
Extension-Manager

Mit dem Extension-Manager können Sie LibreOffice-Extensions hinzufügen, entfernen, deaktivieren, aktivieren und aktualisieren.
note

For security reasons, the installation and removal of extensions are controlled by settings in the Expert Configuration. By default, installation and removal are enabled.
Comment 1 Robert Großkopf 2021-12-17 18:33:16 UTC
Could confirm BaseDocumenter doesn't document internal Firebird databases. Works only with HSQLDB.

Documenting Firebird database quits with 
BASIC-RuntimeError.
Property or method not found: ConnectionInfo.

This part of the code is marked:
SetProperty("JavaDriverClass",

… no wonder.

Tested with LO 7.2.5.1 on OpenSuSE 15.2 64bit rpm Linux.
Comment 2 Julien Nabet 2021-12-21 18:38:13 UTC
On pc Debian x86-64 with master sources updated today, I gave a try.

1) I installed the extension 0.6.0 retrieved from https://extensions.libreoffice.org/en/extensions/show/basedocumenter-to-document-your-base-applications (I chose  "For all users" when installing extension).

2) I created a brand new odb file (HSQLDB embedded) with just 1 table.

3) Menu BaseDocumenter/"New repository
=>  A new (empty) repository for database...
Do you agree to continue
I clicked Yes then selected a location (on local disk) and letting the by default filename BaseDocumenter.odb

=> I got an popup message with just the title "ABORT"

Did I miss something?
Comment 3 Robert Großkopf 2021-12-21 18:53:53 UTC
(In reply to Julien Nabet from comment #2)
> 
> Did I miss something?

Could be you got an extension called basedocumenter.oxt. Seems it must be written BaseDocumenter.oxt. Another bug?
Comment 4 Julien Nabet 2021-12-21 19:18:31 UTC
(In reply to Robert Großkopf from comment #3)
> (In reply to Julien Nabet from comment #2)
> > 
> > Did I miss something?
> 
> Could be you got an extension called basedocumenter.oxt. Seems it must be
> written BaseDocumenter.oxt. Another bug?

No I got BaseDocumenter.oxt with right uppercase/lowercase.
Digging a bit more, I found doc about Access2Base http://www.access2base.com/access2base.html and added DBOpen macro.
But I got an abort.

Jean-Pierre: trying to debug, I got an abort here:
Public Function _CheckArgument(pvItem As Variant _
, ByVal piArgNr As Integer _
, Byval pvType As Variant _
, ByVal Optional pvValid As Variant _
, ByVal Optional pvError As Boolean _
) As Variant
'	Called by public functions to check the validity of their arguments
'pvItem 				Argument to be checked
'piArgNr				Argument sequence number
'pvType				Single value or array of allowed variable types
'	If of string type must contain one or more valid pseudo-object types
'pvValid	Single value or array of allowed values - comparison for strings is case-insensitive
'pvError	If True (default), error handling in this routine. False in _setProperty methods in class modules.

	_CheckArgument = False
	
Dim iVarType As Integer, bValidIsMissing As Boolean
If IsArray(pvType) Then iVarType = VarType(pvType(LBound(pvType))) Else iVarType = VarType(pvType)
If iVarType = vbString Then '	pvType is a pseudo-type string
	_CheckArgument = Utils._IsPseudo(pvItem, pvType)
Else
	bValidIsMissing = ( VarType(pvValid) = vbError ) <==== HERE

I think it's because pvValid is empty

I found this commit: 57666b8ba70f27c7250ef32f4cb9e7660e258521
Access2Base - tdf#136063 Workaround Basic missing argument handling (2)

But reading tdf#136063, it seems a proper fix has been done with tdf#136143.

Shouldn't revert the fix for tdf#136063 ?
Comment 5 Julien Nabet 2021-12-21 20:52:59 UTC
Argh reverting https://cgit.freedesktop.org/libreoffice/core/commit/?id=57666b8ba70f27c7250ef32f4cb9e7660e258521, it fails here:

If IsMissing(pvValid) Then _CheckArgument = Utils._IsScalar(pvItem, pvType) Else _CheckArgument = Utils._IsScalar(pvItem, pvType, pvValid)

In fact, after some debug it fails precisely on
IsMissing(pvValid)

Andreas/Mike: it might be related to tdf#125180
Comment 6 Andreas Heinisch 2021-12-22 07:24:48 UTC
Have to investigate further, but in the function TraceError I get the error message "Argument is not optional" raised at:

Public Function _CheckArgument
...
    bValidIsMissing = ( VarType(pvValid) = vbError )
...

called in:

Public Function OpenDatabase
...
    If Not Utils._CheckArgument(pvDatabaseURL, 1, vbString) Then Goto Exit_Function
...

Strange though that here the iVarType is not vbString:

Dim iVarType As Integer, bValidIsMissing As Boolean
	If IsArray(pvType) Then iVarType = VarType(pvType(LBound(pvType))) Else iVarType = VarType(pvType)
	If iVarType = vbString Then					'	pvType is a pseudo-type string
		_CheckArgument = Utils._IsPseudo(pvItem, pvType)
	Else
		bValidIsMissing = ( VarType(pvValid) = vbError )
		If Not bValidIsMissing Then bValidIsMissing = IsMissing(pvValid)
		If bValidIsMissing Then _CheckArgument = Utils._IsScalar(pvItem, pvType) Else _CheckArgument = Utils._IsScalar(pvItem, pvType, pvValid)
	End If
Comment 7 Mike Kaganski 2021-12-22 07:44:47 UTC
(In reply to Julien Nabet from comment #5)

Debugging _CheckArgument in Basic IDE, I see that *before* the initial assignment '_CheckArgument = False', all the arguments show in the IDE's tooltip (either their values, or <missing parameter>), and also they show in watch window; but after that line, all of them are shown in Watch window as "Out of scope".

Possibly the above is unrelated, and is only some bug in IDE, so please disregard that for now.

Another possibly unrelated thing that I see while debugging is setting an error when calling m_xMethod->Clear() in BasicScriptImpl::invoke [2] (see tdf#143582).

My suspicion wrt the current problem would be SbiRuntime::StepPARAM call when executing [1].

[1] https://opengrok.libreoffice.org/xref/core/wizards/source/access2base/Utils.xba?r=57666b8b&mo=4607&fi=113#113
[2] https://opengrok.libreoffice.org/xref/core/scripting/source/basprov/basscript.cxx?r=24d24deb&mo=8965&fi=247#247
Comment 8 Mike Kaganski 2021-12-22 08:00:02 UTC
(In reply to Mike Kaganski from comment #7)

I actually suspect that standalone parentheses around 'VarType(pvValid) = vbError' are somehow confused by Basic as current function's (_CheckArgument) new (5th) argument, which is set up for unclear reason ... ?

(It's unclear if the revert in comment 5 shows another problem - I didn't revert that yet)
Comment 9 Mike Kaganski 2021-12-22 08:40:57 UTC
Revert shows the same problem, so it's not the parentheses. Possibly it's access to pvValid, that somehow gets wrong index - StepPARAM tries to access param with index 4, which is (having indices 0-based) pvError; it's a variant Boolean, so fixed-type (unlike pvValid) ... and then it breaks somewhere else.

And then - the interesting point is that *the first time* of having this problem, pInfo inside StepPARAM gives correct data for the 4th param, and the Basic code continues past the problematic line - failing elsewhere; but the following executions of this line break because pInfo does not have any info, and then the "argument not optional" problem appears.

Andreas: I stop here, just was curious - please ignore my posts if they are unrelated. Sorry for the noise. :-)
Comment 10 Jean-Pierre Ledure 2021-12-22 10:38:51 UTC
(In reply to Robert Großkopf from comment #1)
> Could confirm BaseDocumenter doesn't document internal Firebird databases.
> Works only with HSQLDB.
> 
> Documenting Firebird database quits with 
> BASIC-RuntimeError.
> Property or method not found: ConnectionInfo.
> 
> This part of the code is marked:
> SetProperty("JavaDriverClass",
> 
> … no wonder.
> 
> Tested with LO 7.2.5.1 on OpenSuSE 15.2 64bit rpm Linux.

Above behaviour is confirmed on:

Version: 7.2.4.1 / LibreOffice Community
Build ID: 27d75539669ac387bb498e35313b970b7fe9c4f9
CPU threads: 6; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: fr-BE (en_US.UTF-8); UI: en-US
Calc: threaded

For the time being, BaseDocumenter expects Java drivers => Firebird is not supported, indeed.
Comment 11 Jean-Pierre Ledure 2021-12-22 10:44:42 UTC
(In reply to Julien Nabet from comment #4)
> (In reply to Robert Großkopf from comment #3)
> > (In reply to Julien Nabet from comment #2)
> > > 
> > > Did I miss something?
> > 
> > Could be you got an extension called basedocumenter.oxt. Seems it must be
> > written BaseDocumenter.oxt. Another bug?
> 
> No I got BaseDocumenter.oxt with right uppercase/lowercase.
> Digging a bit more, I found doc about Access2Base
> http://www.access2base.com/access2base.html and added DBOpen macro.
> But I got an abort.
> 
> Jean-Pierre: trying to debug, I got an abort here:
> Public Function _CheckArgument(pvItem As Variant _
> , ByVal piArgNr As Integer _
> , Byval pvType As Variant _
> , ByVal Optional pvValid As Variant _
> , ByVal Optional pvError As Boolean _
> ) As Variant
> '	Called by public functions to check the validity of their arguments
> 'pvItem 				Argument to be checked
> 'piArgNr				Argument sequence number
> 'pvType				Single value or array of allowed variable types
> '	If of string type must contain one or more valid pseudo-object types
> 'pvValid	Single value or array of allowed values - comparison for strings is
> case-insensitive
> 'pvError	If True (default), error handling in this routine. False in
> _setProperty methods in class modules.
> 
> 	_CheckArgument = False
> 	
> Dim iVarType As Integer, bValidIsMissing As Boolean
> If IsArray(pvType) Then iVarType = VarType(pvType(LBound(pvType))) Else
> iVarType = VarType(pvType)
> If iVarType = vbString Then '	pvType is a pseudo-type string
> 	_CheckArgument = Utils._IsPseudo(pvItem, pvType)
> Else
> 	bValidIsMissing = ( VarType(pvValid) = vbError ) <==== HERE
> 
> I think it's because pvValid is empty
> 
> I found this commit:
> 57666b8ba70f27c7250ef32f4cb9e7660e258521
> Access2Base - tdf#136063 Workaround Basic missing argument handling (2)
> 
> But reading tdf#136063, it seems a proper fix has been done with tdf#136143.
> 
> Shouldn't revert the fix for tdf#136063 ?

I could not reproduce above error in

Version: 7.2.4.1 / LibreOffice Community
Build ID: 27d75539669ac387bb498e35313b970b7fe9c4f9
CPU threads: 6; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: fr-BE (en_US.UTF-8); UI: en-US
Calc: threaded
Comment 12 Andreas Heinisch 2021-12-22 21:12:30 UTC
Since it works with 7.2.4.1 the culprit is not tdf#136143. However, I think it is a regression from tdf#144353 (Assigning a missing optional variable to a property does not trigger "argument is not optional")
Comment 13 Andreas Heinisch 2021-12-22 21:42:02 UTC
Like Miklos pointed out in [1] (especially on slide 4) it is not easy to solve something "trivial" like the optionals which continue to give me headaches :)

However, in [2] we could only copy the pInfo over to the variable if there is a missing parameter. Otherwise, the pInfo should be constructed from scratch. Initially, I thought that pInfo will be used only for methods, and therefore the check !dynamic_cast<const SbxMethod*>(&r) should exclude all of them, but apparently it isn't the case. I will try to come up with a patch, but I can't think of an automated test since the original test in tdf#136143 missed the problem.

[1] https://people.gnome.org/~michael/data/2016-04-29-solving-problems.pdf
[2] https://opengrok.libreoffice.org/xref/core/basic/source/sbx/sbxvar.cxx?r=47aabde0&mo=7146&fi=285#285
Comment 14 Commit Notification 2021-12-27 19:50:25 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d8428094c7f8b186b37c76fd7e9508a075424f80

tdf#144353, tdf#146281 - Correctly copy the information about variables

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Andreas Heinisch 2021-12-27 19:59:18 UTC
Tried to add a test case but I can't recreate the error on a smaller scale. Somehow in the assignment (_CheckArgument = False) the literal gets an empty parameter info and then it will overwrite the information about the parameters in _CheckArgument leading to that error.
Comment 16 Julien Nabet 2021-12-28 18:19:24 UTC
(In reply to Andreas Heinisch from comment #15)
> Tried to add a test case but I can't recreate the error on a smaller scale.
> Somehow in the assignment (_CheckArgument = False) the literal gets an empty
> parameter info and then it will overwrite the information about the
> parameters in _CheckArgument leading to that error.

At least, I can tell that Access2Base works now thanks to your patch!

Jean-Pierre:
I gave a try to BaseDocumenter first with Firebird, it seems the only pb is in BD_Scan.xba
Commenting lines 386 to 389 included prevents from having an error:
386 ' _BD_.DatabaseRecord.SetProperty(&quot;JavaDriverClass&quot;, _
387 '       UtilProperty._GetPropertyValue(.MetaData.ConnectionInfo, &quot;JavaDriverClass&quot;, &quot;&quot;))
388 ' _BD_.DatabaseRecord.SetProperty(&quot;JavaDriverClassPath&quot;, _
389 '       UtilProperty._GetPropertyValue(.MetaData.ConnectionInfo, &quot;JavaDriverClassPath&quot;, &quot;&quot;))

I didn't see the report but I had this same pb with HSQLDB embedded ; I think it's just because I didn't configure something but didn't find a way to do it even after having taken a look at http://www.access2base.com/basedocumenter/basedocumenter.html. Anyway it's another story.
Comment 17 Commit Notification 2022-02-10 08:21:08 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/1a343e3168c6ac8acf3279307275727e588ddcb7

tdf#144353, tdf#146281 - Correctly copy the information about variables

It will be available in 7.3.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.