From 67b51abd659fa975d73c9fc726a90a43cc1dea8d Mon Sep 17 00:00:00 2001 From: Vissarion Fisikopoulos Date: Tue, 12 May 2026 23:03:47 +0300 Subject: [PATCH] tdf#42713: maintain SQL comments in design view When an SQL query has comments then those were deleted when switching to design view. This is fixed and the comments are maintained. In the case the query is edited in design view then the comments are inevitably lost. Change-Id: Ibcf6e36fce88a526c7340d352aca9859b1b3714e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/205441 Tested-by: Jenkins Reviewed-by: Vissarion Fisikopoulos --- .../CppunitTest_connectivity_commontools.mk | 2 + .../commontools/SQLCommentTest.cxx | 408 ++++++++++++++++++ connectivity/source/parse/sqlbison.y | 66 +-- connectivity/source/parse/sqlflex.l | 9 +- dbaccess/qa/uitest/data/tdf42713.odb | Bin 0 -> 3449 bytes .../query/query_comment_preservation.py | 87 ++++ dbaccess/source/ui/inc/querycontroller.hxx | 10 + .../source/ui/querydesign/querycontroller.cxx | 81 +++- 8 files changed, 595 insertions(+), 68 deletions(-) create mode 100644 connectivity/qa/connectivity/commontools/SQLCommentTest.cxx create mode 100644 dbaccess/qa/uitest/data/tdf42713.odb create mode 100644 dbaccess/qa/uitest/query/query_comment_preservation.py diff --git a/connectivity/CppunitTest_connectivity_commontools.mk b/connectivity/CppunitTest_connectivity_commontools.mk index fe87473fd42e..997cdf3a08ce 100644 --- a/connectivity/CppunitTest_connectivity_commontools.mk +++ b/connectivity/CppunitTest_connectivity_commontools.mk @@ -24,6 +24,7 @@ $(eval $(call gb_CppunitTest_use_sdk_api,connectivity_commontools)) $(eval $(call gb_CppunitTest_add_exception_objects,connectivity_commontools, \ connectivity/qa/connectivity/commontools/FValue_test \ connectivity/qa/connectivity/commontools/DBTypeConversion_test \ + connectivity/qa/connectivity/commontools/SQLCommentTest \ )) $(eval $(call gb_CppunitTest_use_libraries,connectivity_commontools, \ @@ -36,6 +37,7 @@ $(eval $(call gb_CppunitTest_use_libraries,connectivity_commontools, \ $(eval $(call gb_CppunitTest_use_components,connectivity_commontools,\ configmgr/source/configmgr \ + i18npool/util/i18npool \ )) $(eval $(call gb_CppunitTest_use_configuration,connectivity_commontools)) diff --git a/connectivity/qa/connectivity/commontools/SQLCommentTest.cxx b/connectivity/qa/connectivity/commontools/SQLCommentTest.cxx new file mode 100644 index 000000000000..b07115bcfc31 --- /dev/null +++ b/connectivity/qa/connectivity/commontools/SQLCommentTest.cxx @@ -0,0 +1,408 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +/** + * Unit tests for SQL comment handling in the connectivity SQL parser + * (OSQLParser / OSQLScanner / OSQLParseNode). + * + * These tests verify that the flex lexer correctly discards all three + * comment styles (-- line, slash-star block star-slash) so that OSQLParser::parseTree() + * succeeds and OSQLParseNode::parseNodeToStr() produces comment-free output. + * + */ + +#include + +#include +#include + +using namespace ::connectivity; + +namespace connectivity::commontools +{ +class SQLCommentTest : public test::BootstrapFixture +{ +public: + SQLCommentTest() + : test::BootstrapFixture(false, false) + { + } + + void setUp() override; + void tearDown() override; + + // -- line comment + void test_dashDash_inline_comment(); + void test_dashDash_comment_at_end_of_input_no_newline(); + void test_dashDash_comment_strips_from_output(); + + // /* block */ comment + void test_block_comment_inline(); + void test_block_comment_strips_from_output(); + void test_multiline_block_comment(); + + // -- in string literals must NOT be treated as a comment + void test_dashDash_inside_string_literal_not_comment(); + + // canonical equivalence: SQL with comment must serialise identically to SQL without + void test_canonical_dashDash_equals_plain(); + void test_canonical_block_equals_plain(); + + // comment in every syntactic position + void test_comment_between_select_and_columns(); + void test_comment_between_columns_and_from(); + void test_comment_in_where_clause(); + + // multiple and adjacent comments + void test_multiple_dashDash_comments(); + void test_mixed_comment_styles(); + + // edge cases + void test_empty_block_comment(); + void test_block_comment_containing_stars(); + void test_slashSlash_inside_string_literal_not_comment(); + + // "//" is not a valid comment; parser must reject it + void test_slashSlash_rejected(); + + // real comment + -- inside string literal in same statement + void test_real_comment_and_dashDash_in_literal_same_statement(); + + CPPUNIT_TEST_SUITE(SQLCommentTest); + + CPPUNIT_TEST(test_dashDash_inline_comment); + CPPUNIT_TEST(test_dashDash_comment_at_end_of_input_no_newline); + CPPUNIT_TEST(test_dashDash_comment_strips_from_output); + + CPPUNIT_TEST(test_block_comment_inline); + CPPUNIT_TEST(test_block_comment_strips_from_output); + CPPUNIT_TEST(test_multiline_block_comment); + + CPPUNIT_TEST(test_dashDash_inside_string_literal_not_comment); + + CPPUNIT_TEST(test_canonical_dashDash_equals_plain); + CPPUNIT_TEST(test_canonical_block_equals_plain); + + CPPUNIT_TEST(test_comment_between_select_and_columns); + CPPUNIT_TEST(test_comment_between_columns_and_from); + CPPUNIT_TEST(test_comment_in_where_clause); + + CPPUNIT_TEST(test_multiple_dashDash_comments); + CPPUNIT_TEST(test_mixed_comment_styles); + + CPPUNIT_TEST(test_empty_block_comment); + CPPUNIT_TEST(test_block_comment_containing_stars); + CPPUNIT_TEST(test_slashSlash_inside_string_literal_not_comment); + + CPPUNIT_TEST(test_slashSlash_rejected); + + CPPUNIT_TEST(test_real_comment_and_dashDash_in_literal_same_statement); + + CPPUNIT_TEST_SUITE_END(); + +private: + /** Count all nodes in the parse tree recursively. */ + static sal_uInt32 countNodes(const OSQLParseNode* pNode); + + std::unique_ptr m_pParser; +}; + +void SQLCommentTest::setUp() +{ + test::BootstrapFixture::setUp(); + // OSQLParser needs a service factory and locale; nullptr is acceptable for + // tests that only exercise comment stripping. + m_pParser = std::make_unique(getComponentContext()); +} + +void SQLCommentTest::tearDown() +{ + m_pParser.reset(); + test::BootstrapFixture::tearDown(); +} + +// Simple check, comments produce no AST nodes +// Workaround since we cannot connect a DB to the parser here + +sal_uInt32 SQLCommentTest::countNodes(const OSQLParseNode* pNode) +{ + if (!pNode) + return 0; + sal_uInt32 n = 1; + for (size_t i = 0; i < pNode->count(); ++i) + n += countNodes(pNode->getChild(i)); + return n; +} + +// --------------------------------------------------------------------------- +// -- line comment tests +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_dashDash_inline_comment() +{ + // "-- comment\n" at end of a valid SQL statement must be consumed. + OUString sSQL(u"SELECT * FROM \"t\" -- inline comment\n"_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must succeed with a trailing -- comment", pNode != nullptr); +} + +void SQLCommentTest::test_dashDash_comment_at_end_of_input_no_newline() +{ + // just to be sure that he new flex rule for "--" works at end-of-input + // without a trailing newline + OUString sSQL(u"SELECT * FROM \"t\" --no newline at end"_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must succeed with -- comment at end-of-input (no newline)", + pNode != nullptr); +} + +void SQLCommentTest::test_dashDash_comment_strips_from_output() +{ + // After parsing, the parser must succeed (i.e. the comment was lexed out). + // Note: parseNodeToStr() needs a real DB connection and cannot be called + // with nullptr; parse success alone is proof the comment was consumed. + OUString sSQL(u"SELECT * FROM \"t\" -- strip me\n"_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must succeed (comment stripped by lexer)", + pNode != nullptr); + CPPUNIT_ASSERT_MESSAGE("No error message expected", sError.isEmpty()); +} + +// --------------------------------------------------------------------------- +// /* block */ comment tests +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_block_comment_inline() +{ + OUString sSQL(u"SELECT /* block comment */ * FROM \"t\""_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must succeed with an inline /* comment */", + pNode != nullptr); +} + +void SQLCommentTest::test_block_comment_strips_from_output() +{ + OUString sSQL(u"SELECT /* strip me */ * FROM \"t\""_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must succeed (block comment consumed by lexer)", + pNode != nullptr); + CPPUNIT_ASSERT_MESSAGE("No error message expected", sError.isEmpty()); +} + +void SQLCommentTest::test_multiline_block_comment() +{ + // A block comment spanning multiple lines must be accepted. + OUString sSQL(u"SELECT /*\n multi\n line\n comment\n*/ * FROM \"t\""_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must succeed with a multi-line /* comment */", + pNode != nullptr); +} + +// --------------------------------------------------------------------------- +// -- inside a string literal is NOT a comment +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_dashDash_inside_string_literal_not_comment() +{ + // The -- inside '...' must not be treated as a comment start. + // If it were, the parser would see a truncated WHERE clause and fail. + OUString sSQL(u"SELECT * FROM \"t\" WHERE \"c\" = 'a--b'"_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("Query with -- inside string literal must parse successfully", + pNode != nullptr); + CPPUNIT_ASSERT_MESSAGE("No error message expected", sError.isEmpty()); +} + +// --------------------------------------------------------------------------- +// SQL with comment should be the same as SQL without +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_canonical_dashDash_equals_plain() +{ + // Comments produce no AST nodes. A statement with a trailing -- comment + // must parse to a tree with the same node count as the same statement + // without any comment. + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\" -- a comment\n"_ustr)); + CPPUNIT_ASSERT_MESSAGE("-- comment statement must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("-- comment must not add AST nodes", countNodes(pPlain.get()), + countNodes(pWithComment.get())); +} + +void SQLCommentTest::test_canonical_block_equals_plain() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT /* mid-query comment */ * FROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("/* */ comment statement must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("/* */ comment must not add AST nodes", countNodes(pPlain.get()), + countNodes(pWithComment.get())); +} + +// --------------------------------------------------------------------------- +// Comment in every syntactic position +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_comment_between_select_and_columns() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT -- pick columns\n * FROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("Comment between SELECT and * must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Comment between SELECT and * must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +void SQLCommentTest::test_comment_between_columns_and_from() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT * /* which table? */ FROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("Comment between * and FROM must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Comment between * and FROM must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +void SQLCommentTest::test_comment_in_where_clause() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\" WHERE \"c\" = 1"_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\" WHERE /* filter */ \"c\" = 1"_ustr)); + CPPUNIT_ASSERT_MESSAGE("Comment inside WHERE clause must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Comment inside WHERE clause must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +// --------------------------------------------------------------------------- +// Multiple and adjacent comments +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_multiple_dashDash_comments() +{ + // Two consecutive line comments; each must be consumed independently. + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT * -- first\n-- second\nFROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("Adjacent -- comments must both parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Adjacent -- comments must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +void SQLCommentTest::test_mixed_comment_styles() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT /* A */ * -- B\nFROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("Mixed comment styles in one statement must parse", + pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Mixed comment styles must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +// --------------------------------------------------------------------------- +// Edge cases +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_empty_block_comment() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT /**/ * FROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("Empty /**/ comment must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Empty /**/ comment must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +void SQLCommentTest::test_block_comment_containing_stars() +{ + // Interior stars must not prematurely close the block comment. + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\""_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT /* * not-end * */ * FROM \"t\""_ustr)); + CPPUNIT_ASSERT_MESSAGE("Block comment with interior stars must parse", pWithComment != nullptr); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Block comment with interior stars must not add AST nodes", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +void SQLCommentTest::test_slashSlash_inside_string_literal_not_comment() +{ + // // inside a quoted string must not start a line comment. + OUString sSQL(u"SELECT * FROM \"t\" WHERE \"c\" = 'http://example.com'"_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("// inside string literal must not be treated as comment", + pNode != nullptr); + CPPUNIT_ASSERT_MESSAGE("No error message expected", sError.isEmpty()); +} + +// --------------------------------------------------------------------------- +// // is not a comment — parser must reject it +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_slashSlash_rejected() +{ + // "//" is not valid SQL comment syntax; the parser must reject it. + OUString sSQL(u"SELECT * FROM \"t\" // not a comment\n"_ustr); + OUString sError; + std::unique_ptr pNode(m_pParser->parseTree(sError, sSQL)); + CPPUNIT_ASSERT_MESSAGE("parseTree() must fail when // appears outside a string", + pNode == nullptr); +} + +// --------------------------------------------------------------------------- +// real comment + -- inside string literal in same statement +// --------------------------------------------------------------------------- + +void SQLCommentTest::test_real_comment_and_dashDash_in_literal_same_statement() +{ + OUString sError; + std::unique_ptr pPlain( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\" WHERE \"c\" = 'a--b'"_ustr)); + std::unique_ptr pWithComment( + m_pParser->parseTree(sError, u"SELECT * FROM \"t\" -- comment\nWHERE \"c\" = 'a--b'"_ustr)); + CPPUNIT_ASSERT_MESSAGE( + "Real -- comment combined with -- inside string literal must parse correctly", + pWithComment != nullptr); + CPPUNIT_ASSERT_MESSAGE("No error expected", sError.isEmpty()); + CPPUNIT_ASSERT_EQUAL_MESSAGE( + "Real -- comment must not add AST nodes (-- in literal is not a comment)", + countNodes(pPlain.get()), countNodes(pWithComment.get())); +} + +CPPUNIT_TEST_SUITE_REGISTRATION(SQLCommentTest); + +} // namespace connectivity::commontools + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/connectivity/source/parse/sqlbison.y b/connectivity/source/parse/sqlbison.y index 9b125768d260..553df4f9f4af 100644 --- a/connectivity/source/parse/sqlbison.y +++ b/connectivity/source/parse/sqlbison.y @@ -4529,67 +4529,6 @@ void OSQLParser::setParseTree(OSQLParseNode* pNewParseTree) m_pParseTree.reset(pNewParseTree); } - -/** Delete all comments in a query. - - See also getComment()/concatComment() implementation for - OQueryController::translateStatement(). - */ -static OUString delComment( const OUString& rQuery ) -{ - // First a quick search if there is any "--" or "//" or "/*", if not then the whole - // copying loop is pointless. - if (rQuery.indexOf("--") < 0 && rQuery.indexOf("//") < 0 && - rQuery.indexOf("/*") < 0) - return rQuery; - - const sal_Unicode* pCopy = rQuery.getStr(); - sal_Int32 nQueryLen = rQuery.getLength(); - bool bIsText1 = false; // "text" - bool bIsText2 = false; // 'text' - bool bComment2 = false; // /* comment */ - bool bComment = false; // -- or // comment - OUStringBuffer aBuf(nQueryLen); - for (sal_Int32 i=0; i < nQueryLen; ++i) - { - if (bComment2) - { - if ((i+1) < nQueryLen) - { - if (pCopy[i]=='*' && pCopy[i+1]=='/') - { - bComment2 = false; - ++i; - } - } - else - { - // comment can't close anymore, actually an error, but... - } - continue; - } - if (pCopy[i] == '\n') - bComment = false; - else if (!bComment) - { - if (pCopy[i] == '\"' && !bIsText2) - bIsText1 = !bIsText1; - else if (pCopy[i] == '\'' && !bIsText1) - bIsText2 = !bIsText2; - if (!bIsText1 && !bIsText2 && (i+1) < nQueryLen) - { - if ((pCopy[i]=='-' && pCopy[i+1]=='-') || (pCopy[i]=='/' && pCopy[i+1]=='/')) - bComment = true; - else if ((pCopy[i]=='/' && pCopy[i+1]=='*')) - bComment2 = true; - } - } - if (!bComment && !bComment2) - aBuf.append( &pCopy[i], 1); - } - return aBuf.makeStringAndClear(); -} - std::unique_ptr OSQLParser::parseTree(OUString& rErrorMessage, const OUString& rStatement, bool bInternational) @@ -4601,12 +4540,9 @@ std::unique_ptr OSQLParser::parseTree(OUString& rErrorMessage, // must be reset setParser(this); - // delete comments before parsing - OUString sTemp = delComment(rStatement); - // defines how to scan s_pScanner->SetRule(OSQLScanner::GetSQLRule()); // initial - s_pScanner->prepareScan(sTemp, m_pContext, bInternational); + s_pScanner->prepareScan(rStatement, m_pContext, bInternational); SQLyylval.pParseNode = nullptr; // SQLyypvt = NULL; diff --git a/connectivity/source/parse/sqlflex.l b/connectivity/source/parse/sqlflex.l index b57b04235572..94237b3797c6 100644 --- a/connectivity/source/parse/sqlflex.l +++ b/connectivity/source/parse/sqlflex.l @@ -119,9 +119,11 @@ static void do_fatal_error(const char* msg) %s PREDICATE_GER %s DATE %s STRING +%x BLOCK_COMMENT %option noyywrap %option never-interactive +%option stack %% ABS {SQL_NEW_KEYWORD(SQL_TOKEN_ABS); } @@ -495,7 +497,12 @@ ZONE {SQL_NEW_KEYWORD(SQL_TOKEN_ZONE); } [ \t\r]+ {} -"--".*$ {} +"--"[^\n]* { /* line comment: consumed; preceding whitespace/comments are grouped with the next token */ } + +"/*" { yy_push_state(BLOCK_COMMENT); } +"*/" { yy_pop_state(); } +\n { } +. { } . {YY_FATAL_ERROR("Invalid symbol"); return SQL_TOKEN_INVALIDSYMBOL;} diff --git a/dbaccess/qa/uitest/data/tdf42713.odb b/dbaccess/qa/uitest/data/tdf42713.odb new file mode 100644 index 0000000000000000000000000000000000000000..1e5b809dae21c6acaeeeb17e6aae139450f043e6 GIT binary patch literal 3449 zcmaJ@2{e@L-+ruvkTt|>PeS%RN)1Y5-zIAf1|wspnL(5+*^;swSt?6*AxqZCPC~Mc zkzTyAuaPCof7FrA_x66*InO!wdCu>;@B3W$Z@CO16qIZLKm!2Duu|*IXAB#s0RTAo zNWTCi0tv@?q2Vwz+7)3B!y!;^QXXy&k|-D!ft5s|;cgBndv_$<4JT;_!@>=JY(_&v z^J6m-{})(ED+d@3b`T|n#*mm890HEDF)DUxlx7>*We=Ed!04PLA9k7UVLTP|c-+F< zgE#IJG@4>@5UOYQdc}KVQQvb|jqkJV5Ua2=zhIutWyU9e-?7CQok%(^KhG{=9;fSi zJWbWrGyi485?8iI^O<`VAJGikfwSQP(VoW3mu*ix8Jo7`lg~{cpUDvPN!NunX-$16 zV^R24r+m7*W_{bF%mJDUvFwYywIZGVxJI_JW&I5mcX5RhLpr2SXsJ{XFP;3hBV3Sk zX&x6d0^hNGe=%>(da`AH-03<8ZqM!j`wkY>_c+6s#@ z&>UxytVN9@?fR9SYtLMj3cbXbGwkQL4?SxnJdCj-$jvEDw9!GXp3>6p_coXv1}0wP zZ&|v|!cOWF4Pwk1GQzJZ)ClKS^;XmMAwGuPsrGi&QL@m|af%x74pw2z`U;aRc?OeP zWVi~gex9vTBpjkeXk3+Zx4p>Rg)z+9l@NPiw@`X9N~kof$15VP;*?GV$KsUZ@I40V zLSZwotQDKq(+kupHNN!K5b{dK4gLtFrI4?S^1M>^4g2f$JHGL1LWL5(mnOUu5*Omh zHYCTsyDI%HDdMinU=luOE)m}DlB$_78&p0c`EaItU_4eSUVZ(x*ele5gr-*hj?^Ui zT7O9hkaEvE7-4lniK5w&8~}JoiAPV85DUlQ5N=Lb$=gWRg=-M(ur$+~4XAY0eXkd1 zc+nzy!QXmj#7k8?cCKj@Jb;YqM565Y&)&6&$dTd^|txM%6pSGOL3-X z!Do@SSrE&5A9~U2x@{4J(%r`+%BU9QXC{2}z^*do%VCz-UM4~>v9i>Q$D3b7_{4}d z)L4BYm>)Swfzp7%BaI)h=wc}IHN#73ob?x-1xPlXgnj_>Ui)wp8P_Q^3 z>I0#zuUJn~u2pg0lkUcqWI>`%g9~D1Z!<#-imOfi|jxZNXMNt*gU+Hxim7QJ0=c_bxF#A*rWG^y3#3_u_Rk=~l=mr)$Mp^Dx%2 z7)pMhi|O)dQ_PV&0;~HpZ8E;%R2fmP9GK;O>K$&xfI1slVys&9rN9xU7h+=~G&mq; zwmAxNF|57;UMdUy&uNimZxpbe!A581+*y0&)+eyvw$ITE#iDgxx&{(Uuun?AJgjDY zlsj+yQZt4T+qDQspVB9~>Q|zd5skUqrIR~UKTGOzxa?~0=d{@miFmXi+& zYTXrSiL_-llNW4xKVpILIBd-%ic8g0eiV&#&|ip zv_}D|o<1Gf6va?nLlZy#SDD9*`BqnDRec4LXi=SvC~21ZT!a^BjbPlEE`aIUfUFdId<%=8+3EWY)*O$*wX9oh@`rFXpU&8`%u zW?Ie-OBH98@pgJ$(p)2Kg-&eo5KJML#f@@O>+%wni=*(`#3XYoAs!`g&I`l+aHXes z#vsLvTCQHZ$Lb<0fun1IEIOb`C+AO`TpHnal9k6t#+zMb!tqmYE8J&fiLielpF|VR zUSqY{+bsT~eVaMT-@K+b^2__Gm}KiYK%cNU-(BtC3A4a|w4%T5d0&tGMcpU+4Q0Px zI{ncAzaXk4)sD_+y(}q<4u!Jnv8kn@WlL_n{#LP^f?r|3>CHMjd2!g-Md+Nj!`cGu zTxEI4LYwo4zhq+ach{$6^c25T}g1yPhzI}Vks^rrrjpK526i0UOO0!8( zXQ=S5mX8#2=i&b9=W^!~+CdGicR%?uG%1BPH)_#dr^*(N6>7;ToeP;BLZ>vBY}m(L z_`+2RmdX!QD_G^hjmf}2sUwnuUq5;->I~^b$)95g*@M{c)+>(r<-5^8a@xFWl)Syl zFI80|r)F1Dl>RVftVmme*b@Loa8Be85o#ThYQPmH49pfr{Y6Y@!_({L+4Q`DS_;MG z{lWg@K0@>%y|J3ZK(o-Iv=IMX8)oW;*|oy;hbW0Wf0m`FTjVT_$$)R+axW0e8{T@+i*k>TA?ydRVr_FpV z6XRr{ajkEg3*A%VoAjyqs_@zY3!sZ zJuTCVXLartJ_>os+fwmlUqX`!udXY1C zZQ*j-Uc0=ROV+2NBHzzW+ylg0%kOzv?4Rqab$~tqgdU*<)s>aKfj6NGj%B>`sT5t? zIA@o?R#fISbu#Y59P}K40m3Yk|Mrb{Kkt1F>SyP@kp%nk>Lnd#R)yKE zIXi(}3~G~|`0)kr3>x|S)OlU*JLVe8m>GA_+|}BPur8;y-{TMgcdeHd<%8?UJKc}F zRJ}R2bFyvhWrkPzPKKK`z>%2W7SJn6yYXPKS8ZXjJab@n5Jb33ao|I8(=sQyNFKzB z)UypCWaMnXe_Lvjz`x~v$-{g05c;KCnphsH%!Abw$4Su&q2OjbW!K91++Tf?7 zJS6{@dpaVwlRVY$Uh0VX?~FT`b4ReC`qR`q)auW3?}!~jV*fS;|G#4ois1-(v`3|J zc-X(MdQcok=q7QFO64c(@L_&`>VslA!a9ld$I>x`Q2n?QEkFyfkn}S;80o SQL; the comment must survive the round-trip + # through the graphical query builder. + sql_text = self._get_sql_text(xQueryController, xToolkit) + self.assertIn("-- my comment", sql_text, + "SQL comment must be visible after Design->SQL toggle; got: " + + sql_text) + + def test_comment_preserved_after_round_trip(self): + """A SQL comment survives a full Design->SQL->Design->SQL round-trip.""" + with self.ui_test.load_file(get_url_for_data_file("tdf42713.odb")) as document: + xToolkit = self._open_comment_query() + + with self.ui_test.open_subcomponent_through_command(".uno:DBQueryEdit") as xQueryFrame: + xQueryController = xQueryFrame.getController() + + # 1st toggle: Design -> SQL + after_first = self._get_sql_text(xQueryController, xToolkit) + self.assertIn("-- my comment", after_first, + "Comment missing after first Design->SQL toggle; got: " + + after_first) + + # 2nd toggle: SQL -> Design (switch back) + self.xUITest.executeCommandForProvider(".uno:DBChangeDesignMode", xQueryController) + xToolkit.waitUntilAllIdlesDispatched() + + # 3rd toggle: Design -> SQL (comment must still be present) + after_round_trip = self._get_sql_text(xQueryController, xToolkit) + self.assertIn("-- my comment", after_round_trip, + "Comment lost after Design->SQL->Design->SQL round-trip; got: " + + after_round_trip) + +# vim: set shiftwidth=4 softtabstop=4 expandtab: diff --git a/dbaccess/source/ui/inc/querycontroller.hxx b/dbaccess/source/ui/inc/querycontroller.hxx index d4347e47d58d..af677db13175 100644 --- a/dbaccess/source/ui/inc/querycontroller.hxx +++ b/dbaccess/source/ui/inc/querycontroller.hxx @@ -61,6 +61,10 @@ namespace dbaui css::uno::Reference< css::sdbcx::XAlterView > m_xAlterView; OUString m_sStatement; // contains the current sql statement + OUString + m_sStatementWithComments; // original SQL with comments, saved when entering Design view + OUString + m_sStatementCanonical; // parseNodeToStr of the parse tree when entering Design view OUString m_sUpdateCatalogName; // catalog for update data OUString m_sUpdateSchemaName; // schema for update data mutable OUString @@ -195,6 +199,12 @@ namespace dbaui */ void impl_setViewMode( ::dbtools::SQLExceptionInfo* _pErrorInfo ); + /** Re-syncs m_sStatementCanonical with what the Design view actually generates + after its columns are fully populated. Must be called after forceInitialView() + or impl_setViewMode() when entering graphical design mode. + */ + void impl_resyncStatementCanonical(); + /// sets m_sStatement, and notifies our respective property change listeners void setStatement_fireEvent( const OUString& _rNewStatement, bool _bFireStatementChange = true ); /// sets the m_bEscapeProcessing member, and notifies our respective property change listeners diff --git a/dbaccess/source/ui/querydesign/querycontroller.cxx b/dbaccess/source/ui/querydesign/querycontroller.cxx index 45a94f4d579d..3f92bf51a9bb 100644 --- a/dbaccess/source/ui/querydesign/querycontroller.cxx +++ b/dbaccess/source/ui/querydesign/querycontroller.cxx @@ -461,7 +461,29 @@ void OQueryController::Execute(sal_uInt16 _nId, const Sequence< PropertyValue >& SQLExceptionInfo aError; try { - setStatement_fireEvent( getContainer()->getStatement() ); + OUString sContainerStatement = getContainer()->getStatement(); + + // When switching Design->SQL, preserve the original SQL with + // comments if the query body was not modified in Design view. + OUString sSavedStatementWithComments; + if (m_bGraphicalDesign && !m_sStatementWithComments.isEmpty() && m_pSqlIterator + && m_pSqlIterator->getParseTree()) + { + // Parse the statement and check if it is the same with the + // cached statement m_sStatementCanonical + OUString sContainerCanonical; + { + OUString sErr; + std::unique_ptr<::connectivity::OSQLParseNode> pTmp( + m_aSqlParser.parseTree(sErr, sContainerStatement, m_bGraphicalDesign)); + if (pTmp) + pTmp->parseNodeToStr(sContainerCanonical, getConnection()); + } + if (sContainerCanonical == m_sStatementCanonical) + sSavedStatementWithComments = m_sStatementWithComments; + } + + setStatement_fireEvent(sContainerStatement); if(m_sStatement.isEmpty() && m_pSqlIterator) { // change the view of the data @@ -502,12 +524,35 @@ void OQueryController::Execute(sal_uInt16 _nId, const Sequence< PropertyValue >& { // change the view of the data m_bGraphicalDesign = !m_bGraphicalDesign; + + if (m_bGraphicalDesign) + { + // save the original SQL (with comments) + // so it can be restored when switching back. + m_sStatementWithComments = m_sStatement; + m_sStatementCanonical.clear(); + m_pSqlIterator->getParseTree()->parseNodeToStr( + m_sStatementCanonical, getConnection()); + } + else + { + m_sStatementWithComments.clear(); + m_sStatementCanonical.clear(); + } + OUString sNewStatement; - m_pSqlIterator->getParseTree()->parseNodeToStr( sNewStatement, getConnection() ); + if (!sSavedStatementWithComments.isEmpty()) + sNewStatement = sSavedStatementWithComments; + else if (m_bGraphicalDesign) + sNewStatement = m_sStatementCanonical; + else + m_pSqlIterator->getParseTree()->parseNodeToStr(sNewStatement, + getConnection()); setStatement_fireEvent( sNewStatement ); getContainer()->SaveUIConfig(); m_vTableConnectionData.clear(); impl_setViewMode( &aError ); + impl_resyncStatementCanonical(); } } } @@ -616,6 +661,23 @@ void OQueryController::impl_showAutoSQLViewError( const css::uno::Any& _rErrorDe showError( aErrorContext ); } +void OQueryController::impl_resyncStatementCanonical() +{ + if (!m_bGraphicalDesign || m_sStatementWithComments.isEmpty()) + return; + OUString sDesignSQL = getContainer()->getStatement(); + if (sDesignSQL.isEmpty()) + return; + OUString sErr; + std::unique_ptr<::connectivity::OSQLParseNode> pTmp( + m_aSqlParser.parseTree(sErr, sDesignSQL, m_bGraphicalDesign)); + if (pTmp) + { + m_sStatementCanonical.clear(); + pTmp->parseNodeToStr(m_sStatementCanonical, getConnection()); + } +} + void OQueryController::impl_setViewMode( ::dbtools::SQLExceptionInfo* _pErrorInfo ) { OSL_PRECOND( getContainer(), "OQueryController::impl_setViewMode: illegal call!" ); @@ -816,6 +878,8 @@ void OQueryController::impl_initialize(const ::comphelper::NamedValueCollection& impl_setViewMode( &aError ); } + impl_resyncStatementCanonical(); + if ( aError.isValid() && bAttemptedGraphicalDesign && !m_bGraphicalDesign ) { // we tried initializing the graphical view, this failed, and we were automatically switched to SQL @@ -1704,6 +1768,19 @@ void OQueryController::impl_reset( const bool i_bForceCurrentControllerSettings setQueryComposer(); OSL_ENSURE(m_pSqlIterator,"No SQLIterator set!"); + // When loading (or re-loading) in Design view, restore the statement (possible with comments) + // but only if the user didn't change the statement while in Design view + // Then parse the statement to canonical form and update the cached statement + if (m_bGraphicalDesign && !m_sStatement.isEmpty() && m_sStatementWithComments.isEmpty()) + { + m_sStatementWithComments = m_sStatement; + if (m_pSqlIterator && m_pSqlIterator->getParseTree()) + { + m_sStatementCanonical.clear(); + m_pSqlIterator->getParseTree()->parseNodeToStr(m_sStatementCanonical, getConnection()); + } + } + getContainer()->setNoneVisibleRow(m_nVisibleRows); }