From d64ee176b71f219d55cdd2e13a2915b879330bec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=C5=BE=20Vajngerl?= Date: Thu, 25 Jun 2026 16:16:19 +0900 Subject: [PATCH] sc: fix the area for a table reference reached through a name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier change only handled the simplest case of a list validation that points at a name wrapping a table reference. A name defined as MyTable[Column] still came up empty, and a table with a header row showed its values shifted up by one, with the header as the first entry. Match the name's leading token against a table reference instead of requiring the whole body to be a single token, so column-qualified references are handled too. Take the area from the reference the compiler already resolved, which is the right column without the header and totals rows, and fall back to the database range with those rows dropped when the reference is not yet compiled. Move the table-reference-to-area lookup into one helper shared by the direct branch and the through-a-name branch. Add tests for a numeric table with a header and for a column-qualified reference. Signed-off-by: Tomaž Vajngerl Change-Id: I105fee3a677d5e982403562adc8f9a4e6bb94a25 Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/5531 Tested-by: Jenkins CPCI Reviewed-by: Mike Kaganski (cherry picked from commit 4c9e6da406be75d90323e1728556a1cd3afbaa09) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210986 Tested-by: Jenkins Reviewed-by: Xisco Fauli --- sc/qa/unit/subsequent_export_test6.cxx | 125 +++++++++++++++++++++++-- sc/source/core/data/validat.cxx | 71 ++++++++++---- 2 files changed, 169 insertions(+), 27 deletions(-) diff --git a/sc/qa/unit/subsequent_export_test6.cxx b/sc/qa/unit/subsequent_export_test6.cxx index 69d3f2559d19..8e20bfef40b9 100644 --- a/sc/qa/unit/subsequent_export_test6.cxx +++ b/sc/qa/unit/subsequent_export_test6.cxx @@ -287,6 +287,9 @@ CPPUNIT_TEST_FIXTURE(ScExportTest6, testTableStyleCustomRoundtripXLSX) CPPUNIT_TEST_FIXTURE(ScExportTest6, testValidationListNameThroughTableRef) { + // A list validation pointing at a name whose body is a table reference used to + // come up empty; the dropdown must list the referenced table column instead. + createScDoc(); ScDocument* pDoc = getScDoc(); CPPUNIT_ASSERT(pDoc); @@ -306,25 +309,131 @@ CPPUNIT_TEST_FIXTURE(ScExportTest6, testValidationListNameThroughTableRef) ScRangeData* pNameRaw = pName.get(); CPPUNIT_ASSERT(pDoc->GetRangeName().insert(std::move(pName))); - ScTokenArray aValTokens(*pDoc); - aValTokens.AddRangeName(pNameRaw->GetIndex(), -1); + ScTokenArray aValidationTokens(*pDoc); + aValidationTokens.AddRangeName(pNameRaw->GetIndex(), -1); - ScValidationData aValData(SC_VALID_LIST, ScConditionMode::Equal, &aValTokens, nullptr, *pDoc, - ScAddress(0, 0, 0)); - const sal_uInt32 nValKey = pDoc->AddValidationEntry(aValData); - CPPUNIT_ASSERT(nValKey != 0); + ScValidationData aValidationData(SC_VALID_LIST, ScConditionMode::Equal, &aValidationTokens, + nullptr, *pDoc, ScAddress(0, 0, 0)); + const sal_uInt32 nValidationKey = pDoc->AddValidationEntry(aValidationData); + CPPUNIT_ASSERT(nValidationKey != 0); ScPatternAttr aPattern(pDoc->getCellAttributeHelper()); - aPattern.GetItemSetWritable().Put(SfxUInt32Item(ATTR_VALIDDATA, nValKey)); + aPattern.GetItemSetWritable().Put(SfxUInt32Item(ATTR_VALIDDATA, nValidationKey)); pDoc->ApplyPattern(3, 3, 0, aPattern); - const ScValidationData* pData = pDoc->GetValidationEntry(nValKey); + const ScValidationData* pData = pDoc->GetValidationEntry(nValidationKey); CPPUNIT_ASSERT(pData); std::vector aStrings; CPPUNIT_ASSERT(pData->FillSelectionList(aStrings, ScAddress(3, 3, 0))); CPPUNIT_ASSERT_EQUAL(size_t(3), aStrings.size()); } +CPPUNIT_TEST_FIXTURE(ScExportTest6, testValidationListNameThroughTableRefWithHeader) +{ + // A list validation pointing at a name that wraps a table reference must show + // the table's data rows, even when the table has a header row and holds + // numbers: the dropdown lists the data values in order, starting at the first. + + createScDoc(); + ScDocument* pDoc = getScDoc(); + CPPUNIT_ASSERT(pDoc); + + pDoc->SetString(ScAddress(1, 0, 0), u"Score"_ustr); + pDoc->SetValue(ScAddress(1, 1, 0), 10.0); + pDoc->SetValue(ScAddress(1, 2, 0), 20.0); + pDoc->SetValue(ScAddress(1, 3, 0), 30.0); + + // Header row plus three data rows; the data part the reference resolves to + // starts one row below the header. + std::unique_ptr pDB(new ScDBData(u"MyTable"_ustr, 0, 1, 0, 1, 3, true, true)); + ScDBData* pDBRaw = pDB.get(); + CPPUNIT_ASSERT(pDoc->GetDBCollection()->getNamedDBs().insert(std::move(pDB))); + + ScTokenArray aNameTokens(*pDoc); + aNameTokens.AddTableRef(pDBRaw->GetIndex()); + auto pName + = std::make_unique(*pDoc, u"MyName"_ustr, aNameTokens, ScAddress(0, 0, 0)); + ScRangeData* pNameRaw = pName.get(); + CPPUNIT_ASSERT(pDoc->GetRangeName().insert(std::move(pName))); + + ScTokenArray aValidationTokens(*pDoc); + aValidationTokens.AddRangeName(pNameRaw->GetIndex(), -1); + + ScValidationData aValidationData(SC_VALID_LIST, ScConditionMode::Equal, &aValidationTokens, + nullptr, *pDoc, ScAddress(0, 0, 0)); + const sal_uInt32 nValidationKey = pDoc->AddValidationEntry(aValidationData); + CPPUNIT_ASSERT(nValidationKey != 0); + + ScPatternAttr aPattern(pDoc->getCellAttributeHelper()); + aPattern.GetItemSetWritable().Put(SfxUInt32Item(ATTR_VALIDDATA, nValidationKey)); + pDoc->ApplyPattern(3, 3, 0, aPattern); + + const ScValidationData* pData = pDoc->GetValidationEntry(nValidationKey); + CPPUNIT_ASSERT(pData); + std::vector aStrings; + CPPUNIT_ASSERT(pData->FillSelectionList(aStrings, ScAddress(3, 3, 0))); + CPPUNIT_ASSERT_EQUAL(size_t(3), aStrings.size()); + CPPUNIT_ASSERT_EQUAL(u"10"_ustr, aStrings[0].GetString()); + CPPUNIT_ASSERT_EQUAL(u"20"_ustr, aStrings[1].GetString()); + CPPUNIT_ASSERT_EQUAL(u"30"_ustr, aStrings[2].GetString()); +} + +CPPUNIT_TEST_FIXTURE(ScExportTest6, testValidationListNameThroughTableRefColumn) +{ + // A name whose body is a column-qualified table reference like MyTable[Score] + // must list only that column's data rows, not the first column and not the + // header. Such a reference compiles to several tokens, which used to leave the + // dropdown empty. + + createScDoc(); + ScDocument* pDoc = getScDoc(); + CPPUNIT_ASSERT(pDoc); + + pDoc->SetString(ScAddress(0, 0, 0), u"Name"_ustr); + pDoc->SetString(ScAddress(1, 0, 0), u"Score"_ustr); + pDoc->SetString(ScAddress(0, 1, 0), u"x"_ustr); + pDoc->SetValue(ScAddress(1, 1, 0), 10.0); + pDoc->SetString(ScAddress(0, 2, 0), u"y"_ustr); + pDoc->SetValue(ScAddress(1, 2, 0), 20.0); + pDoc->SetString(ScAddress(0, 3, 0), u"z"_ustr); + pDoc->SetValue(ScAddress(1, 3, 0), 30.0); + + // Two-column table A1:B4 with a header row. + std::unique_ptr pDB(new ScDBData(u"MyTable"_ustr, 0, 0, 0, 1, 3, true, true)); + CPPUNIT_ASSERT(pDoc->GetDBCollection()->getNamedDBs().insert(std::move(pDB))); + + // Base the name outside the table so the column resolves to the data rows, + // not an intersecting single cell. + auto pName = std::make_unique(*pDoc, u"MyName"_ustr, u"MyTable[[Score]]"_ustr, + ScAddress(0, 5, 0), ScRangeData::Type::Name, + formula::FormulaGrammar::GRAM_NATIVE); + // A column-qualified reference is more than one token. + CPPUNIT_ASSERT(pName->GetCode()->GetLen() > 1); + ScRangeData* pNameRaw = pName.get(); + CPPUNIT_ASSERT(pDoc->GetRangeName().insert(std::move(pName))); + + ScTokenArray aValidationTokens(*pDoc); + aValidationTokens.AddRangeName(pNameRaw->GetIndex(), -1); + + ScValidationData aValidationData(SC_VALID_LIST, ScConditionMode::Equal, &aValidationTokens, + nullptr, *pDoc, ScAddress(0, 0, 0)); + const sal_uInt32 nValidationKey = pDoc->AddValidationEntry(aValidationData); + CPPUNIT_ASSERT(nValidationKey != 0); + + ScPatternAttr aPattern(pDoc->getCellAttributeHelper()); + aPattern.GetItemSetWritable().Put(SfxUInt32Item(ATTR_VALIDDATA, nValidationKey)); + pDoc->ApplyPattern(3, 3, 0, aPattern); + + const ScValidationData* pData = pDoc->GetValidationEntry(nValidationKey); + CPPUNIT_ASSERT(pData); + std::vector aStrings; + CPPUNIT_ASSERT(pData->FillSelectionList(aStrings, ScAddress(3, 3, 0))); + CPPUNIT_ASSERT_EQUAL(size_t(3), aStrings.size()); + CPPUNIT_ASSERT_EQUAL(u"10"_ustr, aStrings[0].GetString()); + CPPUNIT_ASSERT_EQUAL(u"20"_ustr, aStrings[1].GetString()); + CPPUNIT_ASSERT_EQUAL(u"30"_ustr, aStrings[2].GetString()); +} + CPPUNIT_TEST_FIXTURE(ScExportTest6, testPivotTablesWriteRowColumnItems) { createScDoc("xlsx/forum-mso-de-104083.xlsx"); diff --git a/sc/source/core/data/validat.cxx b/sc/source/core/data/validat.cxx index bb6e47f67abd..315970d6f419 100644 --- a/sc/source/core/data/validat.cxx +++ b/sc/source/core/data/validat.cxx @@ -805,6 +805,50 @@ sal_uInt32 lclGetCellFormat( const ScDocument& rDoc, const ScAddress& rPos ) return pPattern->GetNumberFormat( rDoc.GetFormatTable() ); } +/** Fills rRange with the cell area a structured table reference stands for. + Returns false when no area can be determined. */ +bool lclGetTableRefArea(const FormulaToken* pToken, const ScDocument& rDocument, + const ScAddress& rPos, ScRange& rRange) +{ + auto pTableRef = static_cast(pToken); + + // A compiled reference already holds the exact area it resolves to: the + // columns and data rows it selects, lined up with the interpreted values. + if (const FormulaToken* pArea = pTableRef->GetAreaRefRPN()) + { + switch (pArea->GetType()) + { + case svSingleRef: + rRange = ScRange( + static_cast(pArea)->GetSingleRef().toAbs(rDocument, rPos)); + return true; + case svDoubleRef: + rRange = static_cast(pArea)->GetDoubleRef().toAbs(rDocument, rPos); + return true; + default: + return false; + } + } + + // Not compiled yet, so fall back to the database range. + const ScDBData* pDBData + = rDocument.GetDBCollection()->getNamedDBs().findByIndex(pTableRef->GetIndex()); + if (!pDBData) + return false; + + pDBData->GetArea(rRange); + // Every selector but the whole table refers to the data part, so leave out + // the header and totals rows. + if (pTableRef->GetItem() != ScTableRefToken::ALL) + { + if (pDBData->HasHeader()) + rRange.aStart.IncRow(); + if (pDBData->HasTotals()) + rRange.aEnd.IncRow(-1); + } + return true; +} + } // namespace bool ScValidationData::HasSelectionList() const @@ -892,12 +936,8 @@ bool ScValidationData::GetSelectionFromFormula( } else if (eOpCode == ocTableRef) { - auto pTRToken = static_cast(t); - if (const ScDBData* pDBData = rDocument.GetDBCollection()->getNamedDBs().findByIndex(pTRToken->GetIndex())) - { - pDBData->GetArea(aRange); + if (lclGetTableRefArea(t, rDocument, rPos, aRange)) bRef = true; - } } else if (eOpCode == ocName) { @@ -909,22 +949,15 @@ bool ScValidationData::GetSelectionFromFormula( } else if (pName) { - // Defined name wrapping a single ocTableRef. + // The name wraps a table reference. A column-qualified one + // like MyTable[Column] is several tokens led by the table + // reference, so resolve the area from that leading token. const ScTokenArray* pNameCode = pName->GetCode(); - if (pNameCode && pNameCode->GetLen() == 1) + const FormulaToken* pFirst = pNameCode ? pNameCode->FirstToken() : nullptr; + if (pFirst && pFirst->GetOpCode() == ocTableRef + && lclGetTableRefArea(pFirst, rDocument, rPos, aRange)) { - formula::FormulaTokenArrayPlainIterator aNameIter(*pNameCode); - const formula::FormulaToken* tInner = aNameIter.GetNextReferenceOrName(); - if (tInner && tInner->GetOpCode() == ocTableRef) - { - auto pTRToken = static_cast(tInner); - if (const ScDBData* pDBData = rDocument.GetDBCollection() - ->getNamedDBs().findByIndex(pTRToken->GetIndex())) - { - pDBData->GetArea(aRange); - bRef = true; - } - } + bRef = true; } } }