strepsirrhini/office-gobmx
9
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Resolves: tdf#141146 Fix LOOKUP in array with result scalar / single reference

Browse source 
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
This commit is contained in:
Eike Rathke 2021-03-25 12:04:22 +01:00
parent c8c5a06eed
commit 042dbf8312
1 changed files with 73 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

109
sc/source/core/tool/interpr1.cxx
View file

@ -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