summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEike Rathke <erack@redhat.com>2021-03-25 12:04:22 +0100
committerEike Rathke <erack@redhat.com>2021-03-25 16:31:29 +0100
commit042dbf83122b14fd1dd32705c8f8b7d65c22f21b (patch)
treeaa3aa08a84ffee45a95293f2a9fa3a0a1c983c25
parentc8c5a06eed6c22e40bda9787ace5467d551130b6 (diff)
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@redhat.com> Tested-by: Jenkins
-rw-r--r--sc/source/core/tool/interpr1.cxx109
1 files changed, 73 insertions, 36 deletions
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