[Libreoffice-commits] core.git: sc/source
Eike Rathke (via logerrit)
logerrit at kemper.freedesktop.org
Thu Mar 25 15:32:10 UTC 2021
sc/source/core/tool/interpr1.cxx | 109 ++++++++++++++++++++++++++-------------
1 file changed, 73 insertions(+), 36 deletions(-)
New commits:
commit 042dbf83122b14fd1dd32705c8f8b7d65c22f21b
Author: Eike Rathke <erack at redhat.com>
AuthorDate: Thu Mar 25 12:04:22 2021 +0100
Commit: Eike Rathke <erack at redhat.com>
CommitDate: Thu Mar 25 16:31:29 2021 +0100
Resolves: tdf#141146 Fix LOOKUP in array with result scalar / single reference
The returns are identical to the cell range search.
Also, the single reference case is just a special case of range
reference and acts as row vector (as number of rows is not greater
than number of columns), equally extending the passed "range" if
found position is greater than 1.
Extending a result range such leads to the result cell not being
listened to and not acting on its changes, this was always the case for
results of an extended range, and Excel seems to have the same problem.
This is logical because the range cell is unknown in advance, and
certainly we don't want to make LOOKUP() a volatile function being
executed on each change everywhere.
Solutions to this could be:
- create a single cell broadcaster/listener on the fly while pushing the
out-of-band cell result
- this is nasty and modifying the model while interpreting is ugly and
error prone and it's unclear who should destroy such broadcaster if
the query or data range/array changed
- so this is a no-go
- create a range broadcaster/listener for the entire row right of the
cell
- would mean to inspect during listener setup to which parameter of
which OpCode a reference belongs
- really? ...no
- which also doesn't help if a given range is too short and is
extended
- would mean to always extend the listener either as row or column
* doubtable if it is really worth it for this one time off fouled up
Excel behaviour
* or should we rather return an error for out-of-band results?
- but then again for ranges it always worked this like
+ do not advertise, or strongly deprecate such use
+ a result range should have the same length as the search
range/array
Change-Id: Ie903f4491844306d3768ee40bd16786ebe648461
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113085
Reviewed-by: Eike Rathke <erack at redhat.com>
Tested-by: Jenkins
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index 65200917a8cc..6c3b01c6a049 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -6589,10 +6589,9 @@ void ScInterpreter::ScLookup()
SCSIZE nLenMajor = 0; // length of major direction
bool bVertical = true; // whether to lookup vertically or horizontally
- // The third parameter, result array, for double, string and single reference.
+ // The third parameter, result array, double, string and reference.
double fResVal = 0.0;
svl::SharedString aResStr;
- ScAddress aResAdr;
StackVar eResArrayType = svUnknown;
if (nParamCount == 3)
@@ -6614,6 +6613,11 @@ void ScInterpreter::ScLookup()
}
}
break;
+ case svSingleRef:
+ PopSingleRef( nResCol1, nResRow1, nResTab);
+ nResCol2 = nResCol1;
+ nResRow2 = nResRow1;
+ break;
case svMatrix:
case svExternalSingleRef:
case svExternalDoubleRef:
@@ -6640,9 +6644,6 @@ void ScInterpreter::ScLookup()
case svString:
aResStr = GetString();
break;
- case svSingleRef:
- PopSingleRef( aResAdr );
- break;
default:
PushIllegalParameter();
return;
@@ -6788,13 +6789,12 @@ void ScInterpreter::ScLookup()
PushString( aResStr );
break;
case svDoubleRef:
- aResAdr.Set( nResCol1, nResRow1, nResTab);
- [[fallthrough]];
case svSingleRef:
- PushCellResultToken( true, aResAdr, nullptr, nullptr);
+ PushCellResultToken( true, ScAddress( nResCol1, nResRow1, nResTab), nullptr, nullptr);
break;
default:
- OSL_FAIL( "ScInterpreter::ScLookup: unhandled eResArrayType, single value data");
+ assert(!"ScInterpreter::ScLookup: unhandled eResArrayType, single value data");
+ PushIllegalParameter();
}
}
else
@@ -6811,7 +6811,8 @@ void ScInterpreter::ScLookup()
PushCellResultToken( true, aDataAdr, nullptr, nullptr);
break;
default:
- OSL_FAIL( "ScInterpreter::ScLookup: unhandled eDataArrayType, single value data");
+ assert(!"ScInterpreter::ScLookup: unhandled eDataArrayType, single value data");
+ PushIllegalParameter();
}
}
return;
@@ -7002,33 +7003,69 @@ void ScInterpreter::ScLookup()
}
else if (nParamCount == 3)
{
- // result array is cell range.
- ScAddress aAdr;
- aAdr.SetTab(nResTab);
- bool bResVertical = (nResRow2 - nResRow1) > 0;
- if (bResVertical)
+ /* TODO: the entire switch is a copy of the cell range search
+ * result, factor out. */
+ switch (eResArrayType)
{
- SCROW nTempRow = static_cast<SCROW>(nResRow1 + nDelta);
- if (nTempRow > mrDoc.MaxRow())
+ case svDoubleRef:
+ case svSingleRef:
{
- PushDouble(0);
- return;
+ // Use the result array vector. Note that the result array is assumed
+ // to be a vector (i.e. 1-dimensional array).
+
+ ScAddress aAdr;
+ aAdr.SetTab(nResTab);
+ bool bResVertical = (nResRow2 - nResRow1) > 0;
+ if (bResVertical)
+ {
+ SCROW nTempRow = static_cast<SCROW>(nResRow1 + nDelta);
+ if (nTempRow > mrDoc.MaxRow())
+ {
+ PushDouble(0);
+ return;
+ }
+ aAdr.SetCol(nResCol1);
+ aAdr.SetRow(nTempRow);
+ }
+ else
+ {
+ SCCOL nTempCol = static_cast<SCCOL>(nResCol1 + nDelta);
+ if (nTempCol > mrDoc.MaxCol())
+ {
+ PushDouble(0);
+ return;
+ }
+ aAdr.SetCol(nTempCol);
+ aAdr.SetRow(nResRow1);
+ }
+ PushCellResultToken( true, aAdr, nullptr, nullptr);
}
- aAdr.SetCol(nResCol1);
- aAdr.SetRow(nTempRow);
- }
- else
- {
- SCCOL nTempCol = static_cast<SCCOL>(nResCol1 + nDelta);
- if (nTempCol > mrDoc.MaxCol())
+ break;
+ case svDouble:
+ case svString:
{
- PushDouble(0);
- return;
+ if (nDelta != 0)
+ PushNA();
+ else
+ {
+ switch (eResArrayType)
+ {
+ case svDouble:
+ PushDouble( fResVal );
+ break;
+ case svString:
+ PushString( aResStr );
+ break;
+ default:
+ ; // nothing
+ }
+ }
}
- aAdr.SetCol(nTempCol);
- aAdr.SetRow(nResRow1);
+ break;
+ default:
+ assert(!"ScInterpreter::ScLookup: unhandled eResArrayType, array search");
+ PushIllegalParameter();
}
- PushCellResultToken(true, aAdr, nullptr, nullptr);
}
else
{
@@ -7094,9 +7131,12 @@ void ScInterpreter::ScLookup()
}
else if (nParamCount == 3)
{
+ /* TODO: the entire switch is a copy of the array search result, factor
+ * out. */
switch (eResArrayType)
{
case svDoubleRef:
+ case svSingleRef:
{
// Use the result array vector. Note that the result array is assumed
// to be a vector (i.e. 1-dimensional array).
@@ -7131,7 +7171,6 @@ void ScInterpreter::ScLookup()
break;
case svDouble:
case svString:
- case svSingleRef:
{
if (nDelta != 0)
PushNA();
@@ -7145,9 +7184,6 @@ void ScInterpreter::ScLookup()
case svString:
PushString( aResStr );
break;
- case svSingleRef:
- PushCellResultToken( true, aResAdr, nullptr, nullptr);
- break;
default:
; // nothing
}
@@ -7155,7 +7191,8 @@ void ScInterpreter::ScLookup()
}
break;
default:
- OSL_FAIL( "ScInterpreter::ScLookup: unhandled eResArrayType, range search");
+ assert(!"ScInterpreter::ScLookup: unhandled eResArrayType, range search");
+ PushIllegalParameter();
}
}
else
More information about the Libreoffice-commits
mailing list