Skip to content

Commit 1d6779d

Browse files
sanikolaevsanikolaev
committed
fix: Enhance update validation by checking attributes against full-text fields and filtering valid agents before remote tasks execution in searchd.
Show a warning if an attribute is updated when it's a full-text field too. Clarified an error when updating a full-text field Related issue #3478
1 parent 75f2b34 commit 1d6779d

File tree

6 files changed

+65
-10
lines changed

6 files changed

+65
-10
lines changed

‎src/searchd.cpp‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7250,10 +7250,33 @@ void sphHandleMysqlUpdate ( StmtErrorReporter_i & tOut, const SqlStmt_t & tStmt,
72507250
VecRefPtrs_t<AgentConn_t *> dAgents;
72517251
pDist->GetAllHosts ( dAgents );
72527252

7253+
// validate update before sending to remote agents
7254+
VecRefPtrs_t<AgentConn_t *> dValidAgents;
7255+
for ( AgentConn_t * pAgent : dAgents )
7256+
{
7257+
const CSphString & sRemoteIndex = pAgent->m_tDesc.m_sIndexes;
7258+
auto pRemoteServed = GetServed ( sRemoteIndex );
7259+
if ( pRemoteServed )
7260+
{
7261+
const ISphSchema& tSchema = RIdx_c ( pRemoteServed )->GetMatchSchema();
7262+
CSphString sError;
7263+
if ( !Update_CheckAttributes ( *tStmt.AttrUpdatePtr(), tSchema, sError ) )
7264+
{
7265+
dFails.Submit ( sRemoteIndex, sReqIndex, sError.cstr() );
7266+
continue; // skip this agent
7267+
}
7268+
}
7269+
dValidAgents.Add ( pAgent );
7270+
}
7271+
dAgents = std::move ( dValidAgents );
7272+
72537273
// connect to remote agents and query them
7254-
std::unique_ptr<RequestBuilder_i> pRequestBuilder = CreateRequestBuilder ( sQuery, tStmt );
7255-
std::unique_ptr<ReplyParser_i> pReplyParser = CreateReplyParser ( tStmt.m_bJson, iUpdated, iWarns, dFails );
7256-
iSuccesses += PerformRemoteTasks ( dAgents, pRequestBuilder.get (), pReplyParser.get () );
7274+
if ( !dAgents.IsEmpty() )
7275+
{
7276+
std::unique_ptr<RequestBuilder_i> pRequestBuilder = CreateRequestBuilder ( sQuery, tStmt );
7277+
std::unique_ptr<ReplyParser_i> pReplyParser = CreateReplyParser ( tStmt.m_bJson, iUpdated, iWarns, dFails );
7278+
iSuccesses += PerformRemoteTasks ( dAgents, pRequestBuilder.get (), pReplyParser.get () );
7279+
}
72577280
}
72587281
}
72597282

@@ -7272,6 +7295,7 @@ void sphHandleMysqlUpdate ( StmtErrorReporter_i & tOut, const SqlStmt_t & tStmt,
72727295
LogSphinxqlClause ( sQuery, (int)( tmRealTimeMs ) );
72737296
}
72747297

7298+
iWarns = sWarning.IsEmpty() ? 0 : 1;
72757299
tOut.Ok ( iUpdated, iWarns );
72767300
}
72777301

‎src/searchdreplication.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1360,7 +1360,9 @@ static bool ValidateUpdate ( const ReplicationCommand_t & tCmd )
13601360

13611361
assert ( tCmd.m_pUpdateAPI );
13621362
CSphString sError;
1363-
return Update_CheckAttributes ( *tCmd.m_pUpdateAPI, tSchema, sError ) || TlsMsg::Err ( sError );
1363+
if ( !Update_CheckAttributes ( *tCmd.m_pUpdateAPI, tSchema, sError ) )
1364+
return TlsMsg::Err ( sError );
1365+
return true;
13641366
}
13651367

13661368
// load indexes received from another node or existed already into daemon

‎src/sphinx.cpp‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -664,26 +664,43 @@ UpdateContext_t::UpdateContext_t ( AttrUpdateInc_t & tUpd, const ISphSchema & tS
664664

665665
//////////////////////////////////////////////////////////////////////////
666666

667-
bool Update_CheckAttributes ( const CSphAttrUpdate & tUpd, const ISphSchema & tSchema, CSphString & sError )
667+
bool Update_CheckAttributes ( const CSphAttrUpdate & tUpd, const ISphSchema & tSchema, CSphString & sError, CSphString* pWarning )
668668
{
669669
for ( const auto & tUpdAttr : tUpd.m_dAttributes )
670670
{
671671
const CSphString & sUpdAttrName = tUpdAttr.m_sName;
672+
bool bIsField = ( tSchema.GetField ( sUpdAttrName.cstr() ) != nullptr );
672673
int iUpdAttrId = tSchema.GetAttrIndex ( sUpdAttrName.cstr() );
673674

674675
// try to find JSON attribute with a field
675676
if ( iUpdAttrId<0 )
676677
{
677678
CSphString sJsonCol;
678679
if ( sphJsonNameSplit ( sUpdAttrName.cstr(), nullptr, &sJsonCol ) )
680+
{
681+
bool bJsonColIsField = ( tSchema.GetField ( sJsonCol.cstr() ) != nullptr );
679682
iUpdAttrId = tSchema.GetAttrIndex ( sJsonCol.cstr() );
683+
// if JSON column is a field but not an attribute, reject
684+
if ( bJsonColIsField && iUpdAttrId<0 )
685+
{
686+
sError.SetSprintf ( "attribute '%s' can not be updated (full-text field)", sUpdAttrName.cstr() );
687+
return false;
688+
}
689+
}
680690
}
681691

682692
if ( iUpdAttrId<0 )
683693
{
684694
if ( tUpd.m_bIgnoreNonexistent )
685695
continue;
686696

697+
// if it's a field but not an attribute, reject
698+
if ( bIsField )
699+
{
700+
sError.SetSprintf ( "attribute '%s' can not be updated (full-text field)", sUpdAttrName.cstr() );
701+
return false;
702+
}
703+
687704
sError.SetSprintf ( "attribute '%s' not found", sUpdAttrName.cstr() );
688705
return false;
689706
}
@@ -702,6 +719,16 @@ bool Update_CheckAttributes ( const CSphAttrUpdate & tUpd, const ISphSchema & tS
702719
case SPH_ATTR_BIGINT:
703720
case SPH_ATTR_FLOAT:
704721
case SPH_ATTR_JSON:
722+
// if attribute is also a full-text field, allow update but warn
723+
if ( bIsField && pWarning )
724+
{
725+
CSphString sMsg;
726+
sMsg.SetSprintf ( "attribute '%s' is updated, but full-text field is not (recommended to use REPLACE instead)", sUpdAttrName.cstr() );
727+
if ( pWarning->IsEmpty() )
728+
*pWarning = sMsg;
729+
else
730+
pWarning->SetSprintf ( "%s; %s", pWarning->cstr(), sMsg.cstr() );
731+
}
705732
break;
706733
default:
707734
sError.SetSprintf ( "attribute '%s' can not be updated (must be boolean, integer, bigint, float, timestamp, string, MVA or JSON)", sUpdAttrName.cstr() );
@@ -2468,7 +2495,7 @@ bool CSphIndex_VLN::DoUpdateAttributes ( const RowsToUpdate_t& dRows, UpdateCont
24682495
if ( dRows.IsEmpty() )
24692496
return true;
24702497

2471-
if ( !Update_CheckAttributes ( *tCtx.m_tUpd.m_pUpdate, tCtx.m_tSchema, sError ) )
2498+
if ( !Update_CheckAttributes ( *tCtx.m_tUpd.m_pUpdate, tCtx.m_tSchema, sError, nullptr ) )
24722499
return false;
24732500

24742501
tCtx.m_pHistograms = m_pHistograms;
@@ -2564,9 +2591,11 @@ int CSphIndex_VLN::CheckThenUpdateAttributes ( AttrUpdateInc_t& tUpd, bool& bCri
25642591
if ( !m_iDocinfo || tUpd.m_pUpdate->m_dDocids.IsEmpty() )
25652592
return 0;
25662593

2567-
UpdateContext_t tCtx ( tUpd, m_tSchema );
25682594
int iUpdated = tUpd.m_uAffected;
2595+
if ( !Update_CheckAttributes ( *tUpd.m_pUpdate, m_tSchema, sError, &sWarning ) )
2596+
return -1;
25692597

2598+
UpdateContext_t tCtx ( tUpd, m_tSchema );
25702599
auto dRowsToUpdate = Update_CollectRowPtrs ( tCtx );
25712600

25722601
if ( !DoUpdateAttributes ( dRowsToUpdate, tCtx, bCritical, sError ))

‎src/sphinx.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,7 @@ class IndexSegment_c
10741074
CSphVector<PostponedUpdate_t> m_dPostponedUpdates;
10751075
};
10761076

1077-
bool Update_CheckAttributes ( const CSphAttrUpdate& tUpd, const ISphSchema& tSchema, CSphString& sError );
1077+
bool Update_CheckAttributes ( const CSphAttrUpdate& tUpd, const ISphSchema& tSchema, CSphString& sError, CSphString* pWarning = nullptr );
10781078

10791079
// helper - collects killed documents
10801080
struct KillAccum_t final : public IndexSegment_c

‎src/sphinxrt.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8769,7 +8769,7 @@ int RtIndex_c::CheckThenUpdateAttributes ( AttrUpdateInc_t& tUpd, bool& bCritica
87698769
return 0;
87708770

87718771
int iUpdated = tUpd.m_uAffected;
8772-
if ( !Update_CheckAttributes ( *tUpd.m_pUpdate, m_tSchema, sError ) )
8772+
if ( !Update_CheckAttributes ( *tUpd.m_pUpdate, m_tSchema, sError, &sWarning ) )
87738773
return -1;
87748774

87758775
UpdateContext_t tCtx ( tUpd, m_tSchema );

‎test/clt-tests/http-interface/error-handling.rec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,4 @@ curl -s "http://localhost:9308/cli_json" -d "SELECT * FROM t LIMIT -5"; echo
4040
––– input –––
4141
curl -s "http://localhost:9308/cli_json" -d "UPDATE t SET value='new_value' WHERE id=1"; echo
4242
––– output –––
43-
{"error":"table t: attribute 'value' not found"}
43+
{"error":"table t: attribute 'value' can not be updated (full-text field)"}

0 commit comments

Comments
 (0)