Skip to content

Commit a7b8cdd

Browse files
rouaultrouault
authored
Merge pull request #13842 from rouault/fix_13838
pipeline: make it possible to specify non-first dataset as the result of previous step using ``_PIPE_`` placeholder dataset name
2 parents 2acbd45 + eb01e27 commit a7b8cdd

32 files changed

+578
-165
lines changed

‎apps/gdalalg_abstract_pipeline.cpp‎

Lines changed: 142 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,8 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
385385
};
386386

387387
const auto SetCurStepAlg =
388-
[this, bIsGenericPipeline](Step &curStep, const std::string &algName)
388+
[this, bIsGenericPipeline](Step &curStep, const std::string &algName,
389+
bool firstStep)
389390
{
390391
if (bIsGenericPipeline)
391392
{
@@ -410,6 +411,10 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
410411
algName.c_str());
411412
return false;
412413
}
414+
// We don't want to accept '_PIPE_' dataset placeholder for the first
415+
// step of a pipeline.
416+
curStep.alg->m_inputDatasetCanBeOmitted =
417+
!firstStep || !m_bExpectReadStep;
413418
curStep.alg->SetCallPath({algName});
414419
curStep.alg->SetReferencePathForRelativePaths(
415420
GetReferencePathForRelativePaths());
@@ -533,7 +538,7 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
533538
else if (arg.find("+gdal=") == 0)
534539
{
535540
const std::string algName = arg.substr(strlen("+gdal="));
536-
if (!SetCurStepAlg(curStep, algName))
541+
if (!SetCurStepAlg(curStep, algName, steps.size() == 1))
537542
return false;
538543
}
539544
#endif
@@ -544,7 +549,7 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
544549
if (!algName.empty() && algName[0] == '+')
545550
algName = algName.substr(1);
546551
#endif
547-
if (!SetCurStepAlg(curStep, algName))
552+
if (!SetCurStepAlg(curStep, algName, steps.size() == 1))
548553
return false;
549554
}
550555
else
@@ -562,6 +567,8 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
562567
return false;
563568
}
564569
curStep.isSubAlgorithm = true;
570+
subAlg->m_inputDatasetCanBeOmitted =
571+
steps.size() > 1 || !m_bExpectReadStep;
565572
curStep.alg = std::move(subAlg);
566573
continue;
567574
}
@@ -609,6 +616,7 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
609616
steps.back().alg = GetStepAlg(
610617
std::string(GDALRasterWriteAlgorithm::NAME)
611618
.append(bIsGenericPipeline ? RASTER_SUFFIX : ""));
619+
steps.back().alg->m_inputDatasetCanBeOmitted = true;
612620
}
613621

614622
// Remove "--output-format=stream" and "streamed_dataset" if found
@@ -1047,6 +1055,8 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
10471055
(nDatasetType == 0 || nDatasetType == GDAL_OF_VECTOR))
10481056
{
10491057
step.alg = std::move(algVector);
1058+
step.alg->m_inputDatasetCanBeOmitted =
1059+
!firstStep || !m_bExpectReadStep;
10501060
step.alg->m_skipValidationInParseCommandLine = true;
10511061
ret = step.alg->ParseCommandLineArguments(step.args);
10521062
if (ret)
@@ -1240,6 +1250,8 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
12401250
SetWriteArgFromPipeline();
12411251
}
12421252

1253+
steps[i].alg->m_inputDatasetCanBeOmitted =
1254+
i > 0 || !m_bExpectReadStep;
12431255
steps[i].alg->m_skipValidationInParseCommandLine = true;
12441256
if (!steps[i].alg->ParseCommandLineArguments(steps[i].args))
12451257
return false;
@@ -1253,21 +1265,68 @@ bool GDALAbstractPipelineAlgorithm::ParseCommandLineArguments(
12531265
else if (i > 0 &&
12541266
steps[i].alg->GetInputType() != nLastStepOutputType)
12551267
{
1256-
ReportError(
1257-
CE_Failure, CPLE_AppDefined,
1258-
"Step '%s' expects a %s input dataset, but "
1259-
"previous step '%s' "
1260-
"generates a %s output dataset",
1261-
steps[i].alg->GetName().c_str(),
1262-
steps[i].alg->GetInputType() == GDAL_OF_RASTER ? "raster"
1263-
: steps[i].alg->GetInputType() == GDAL_OF_VECTOR
1264-
? "vector"
1265-
: "unknown",
1266-
steps[i - 1].alg->GetName().c_str(),
1267-
nLastStepOutputType == GDAL_OF_RASTER ? "raster"
1268-
: nLastStepOutputType == GDAL_OF_VECTOR ? "vector"
1269-
: "unknown");
1270-
return false;
1268+
bool emitError = true;
1269+
1270+
// Check if a dataset argument, which has as value the
1271+
// placeholder value, has the same dataset type as the output
1272+
// of the last step
1273+
for (const auto &arg : steps[i].alg->GetArgs())
1274+
{
1275+
if (!arg->IsOutput() &&
1276+
(arg->GetType() == GAAT_DATASET ||
1277+
arg->GetType() == GAAT_DATASET_LIST))
1278+
{
1279+
if (arg->GetType() == GAAT_DATASET)
1280+
{
1281+
if (arg->Get<GDALArgDatasetValue>().GetName() ==
1282+
GDAL_DATASET_PIPELINE_PLACEHOLDER_VALUE)
1283+
{
1284+
if ((arg->GetDatasetType() &
1285+
nLastStepOutputType) != 0)
1286+
{
1287+
emitError = false;
1288+
break;
1289+
}
1290+
}
1291+
}
1292+
else
1293+
{
1294+
CPLAssert(arg->GetType() == GAAT_DATASET_LIST);
1295+
auto &val =
1296+
arg->Get<std::vector<GDALArgDatasetValue>>();
1297+
if (val.size() == 1 &&
1298+
val[0].GetName() ==
1299+
GDAL_DATASET_PIPELINE_PLACEHOLDER_VALUE)
1300+
{
1301+
if ((arg->GetDatasetType() &
1302+
nLastStepOutputType) != 0)
1303+
{
1304+
emitError = false;
1305+
break;
1306+
}
1307+
}
1308+
}
1309+
}
1310+
}
1311+
if (emitError)
1312+
{
1313+
ReportError(CE_Failure, CPLE_AppDefined,
1314+
"Step '%s' expects a %s input dataset, but "
1315+
"previous step '%s' "
1316+
"generates a %s output dataset",
1317+
steps[i].alg->GetName().c_str(),
1318+
steps[i].alg->GetInputType() == GDAL_OF_RASTER
1319+
? "raster"
1320+
: steps[i].alg->GetInputType() == GDAL_OF_VECTOR
1321+
? "vector"
1322+
: "unknown",
1323+
steps[i - 1].alg->GetName().c_str(),
1324+
nLastStepOutputType == GDAL_OF_RASTER ? "raster"
1325+
: nLastStepOutputType == GDAL_OF_VECTOR
1326+
? "vector"
1327+
: "unknown");
1328+
return false;
1329+
}
12711330
}
12721331
nLastStepOutputType = steps[i].alg->GetOutputType();
12731332
}
@@ -1953,18 +2012,75 @@ bool GDALAbstractPipelineAlgorithm::RunStep(GDALPipelineStepRunContext &ctxt)
19532012
auto &step = m_steps[i];
19542013
if (i > 0 || poCurDS)
19552014
{
1956-
if (!step->m_inputDataset.empty() &&
1957-
step->m_inputDataset[0].GetDatasetRef())
2015+
bool prevStepOutputSetToThisStep = false;
2016+
for (auto &arg : step->GetArgs())
2017+
{
2018+
if (!arg->IsOutput() && (arg->GetType() == GAAT_DATASET ||
2019+
arg->GetType() == GAAT_DATASET_LIST))
2020+
{
2021+
if (arg->GetType() == GAAT_DATASET)
2022+
{
2023+
if ((arg->GetName() == GDAL_ARG_NAME_INPUT &&
2024+
!arg->IsExplicitlySet()) ||
2025+
arg->Get<GDALArgDatasetValue>().GetName() ==
2026+
GDAL_DATASET_PIPELINE_PLACEHOLDER_VALUE)
2027+
{
2028+
auto &val = arg->Get<GDALArgDatasetValue>();
2029+
if (val.GetDatasetRef())
2030+
{
2031+
// Shouldn't happen
2032+
ReportError(CE_Failure, CPLE_AppDefined,
2033+
"Step nr %d (%s) has already an "
2034+
"input dataset for argument %s",
2035+
static_cast<int>(i),
2036+
step->GetName().c_str(),
2037+
arg->GetName().c_str());
2038+
return false;
2039+
}
2040+
prevStepOutputSetToThisStep = true;
2041+
val.Set(poCurDS);
2042+
arg->NotifyValueSet();
2043+
}
2044+
}
2045+
else
2046+
{
2047+
CPLAssert(arg->GetType() == GAAT_DATASET_LIST);
2048+
auto &val =
2049+
arg->Get<std::vector<GDALArgDatasetValue>>();
2050+
if ((arg->GetName() == GDAL_ARG_NAME_INPUT &&
2051+
!arg->IsExplicitlySet()) ||
2052+
(val.size() == 1 &&
2053+
val[0].GetName() ==
2054+
GDAL_DATASET_PIPELINE_PLACEHOLDER_VALUE))
2055+
{
2056+
if (val.size() == 1 && val[0].GetDatasetRef())
2057+
{
2058+
// Shouldn't happen
2059+
ReportError(CE_Failure, CPLE_AppDefined,
2060+
"Step nr %d (%s) has already an "
2061+
"input dataset for argument %s",
2062+
static_cast<int>(i),
2063+
step->GetName().c_str(),
2064+
arg->GetName().c_str());
2065+
return false;
2066+
}
2067+
prevStepOutputSetToThisStep = true;
2068+
val.clear();
2069+
val.resize(1);
2070+
val[0].Set(poCurDS);
2071+
arg->NotifyValueSet();
2072+
}
2073+
}
2074+
}
2075+
}
2076+
if (!prevStepOutputSetToThisStep)
19582077
{
1959-
// Shouldn't happen
19602078
ReportError(CE_Failure, CPLE_AppDefined,
1961-
"Step nr %d (%s) has already an input dataset",
2079+
"Step nr %d (%s) does not use input dataset from "
2080+
"previous step",
19622081
static_cast<int>(i), step->GetName().c_str());
19632082
return false;
19642083
}
1965-
step->m_inputDataset.clear();
1966-
step->m_inputDataset.resize(1);
1967-
step->m_inputDataset[0].Set(poCurDS);
19682084
}
19692085

19702086
if (i + 1 < m_steps.size() && step->m_outputDataset.GetDatasetRef() &&
@@ -2007,6 +2123,7 @@ bool GDALAbstractPipelineAlgorithm::RunStep(GDALPipelineStepRunContext &ctxt)
20072123
{
20082124
step->m_stdout = true;
20092125
}
2126+
step->m_inputDatasetCanBeOmitted = false;
20102127
if (!step->ValidateArguments() || !step->RunStep(stepCtxt))
20112128
{
20122129
ret = false;

‎apps/gdalalg_abstract_pipeline.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ class GDALPipelineStepAlgorithm /* non final */ : public GDALAlgorithm
140140
bool addDefaultArguments = true;
141141
bool autoOpenInputDatasets = true;
142142
bool inputDatasetRequired = true;
143+
bool inputDatasetPositional = true;
143144
bool outputDatasetRequired = true;
144145
bool addInputLayerNameArgument = true; // only for vector input
145146
bool addUpdateArgument = true; // only for vector output
@@ -179,6 +180,12 @@ class GDALPipelineStepAlgorithm /* non final */ : public GDALAlgorithm
179180
return *this;
180181
}
181182

183+
inline ConstructorOptions &SetInputDatasetPositional(bool b)
184+
{
185+
inputDatasetPositional = b;
186+
return *this;
187+
}
188+
182189
inline ConstructorOptions &SetInputDatasetMaxCount(int maxCount)
183190
{
184191
inputDatasetMaxCount = maxCount;
@@ -361,6 +368,7 @@ class GDALPipelineStepAlgorithm /* non final */ : public GDALAlgorithm
361368
void AddRasterHiddenInputDatasetArg();
362369

363370
void AddVectorInputArgs(bool hiddenForCLI);
371+
void AddVectorHiddenInputDatasetArg();
364372
void AddVectorOutputArgs(bool hiddenForCLI,
365373
bool shortNameOutputLayerAllowed);
366374
using GDALAlgorithm::AddOutputLayerNameArg;

‎apps/gdalalg_materialize.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ GDALMaterializeRasterAlgorithm::GDALMaterializeRasterAlgorithm()
2929
: GDALMaterializeStepAlgorithm<GDALRasterPipelineStepAlgorithm,
3030
GDAL_OF_RASTER>(HELP_URL)
3131
{
32+
AddRasterHiddenInputDatasetArg();
33+
3234
AddOutputDatasetArg(&m_outputDataset, GDAL_OF_RASTER,
3335
/* positionalAndRequired = */ false,
3436
_("Materialized dataset name"))
@@ -159,6 +161,8 @@ GDALMaterializeVectorAlgorithm::GDALMaterializeVectorAlgorithm()
159161
: GDALMaterializeStepAlgorithm<GDALVectorPipelineStepAlgorithm,
160162
GDAL_OF_VECTOR>(HELP_URL)
161163
{
164+
AddVectorHiddenInputDatasetArg();
165+
162166
AddOutputDatasetArg(&m_outputDataset, GDAL_OF_VECTOR,
163167
/* positionalAndRequired = */ false,
164168
_("Materialized dataset name"))

‎apps/gdalalg_pipeline.cpp‎

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,8 @@ GDALPipelineStepAlgorithm::GDALPipelineStepAlgorithm(
6262

6363
void GDALPipelineStepAlgorithm::AddRasterHiddenInputDatasetArg()
6464
{
65-
// Added so that "band" argument validation works, because
66-
// GDALAlgorithm must be able to retrieve the input dataset
67-
AddInputDatasetArg(&m_inputDataset, GDAL_OF_RASTER,
68-
/* positionalAndRequired = */ false)
69-
.SetMinCount(1)
65+
AddInputDatasetArg(&m_inputDataset, GDAL_OF_RASTER, false)
66+
.SetMinCount(0)
7067
.SetMaxCount(m_constructorOptions.inputDatasetMaxCount)
7168
.SetAutoOpenDataset(m_constructorOptions.autoOpenInputDatasets)
7269
.SetMetaVar(m_constructorOptions.inputDatasetMetaVar)
@@ -93,13 +90,16 @@ void GDALPipelineStepAlgorithm::AddRasterInputArgs(
9390
&m_inputDataset,
9491
openForMixedRasterVector ? (GDAL_OF_RASTER | GDAL_OF_VECTOR)
9592
: GDAL_OF_RASTER,
96-
m_constructorOptions.inputDatasetRequired && !hiddenForCLI,
97-
m_constructorOptions.inputDatasetHelpMsg.c_str())
98-
.SetMinCount(1)
93+
false, m_constructorOptions.inputDatasetHelpMsg.c_str())
94+
.SetMinCount(m_constructorOptions.inputDatasetRequired ? 1 : 0)
9995
.SetMaxCount(m_constructorOptions.inputDatasetMaxCount)
10096
.SetAutoOpenDataset(m_constructorOptions.autoOpenInputDatasets)
10197
.SetMetaVar(m_constructorOptions.inputDatasetMetaVar)
10298
.SetHiddenForCLI(hiddenForCLI);
99+
if (m_constructorOptions.inputDatasetPositional && !hiddenForCLI)
100+
arg.SetPositional();
101+
if (m_constructorOptions.inputDatasetRequired && !hiddenForCLI)
102+
arg.SetRequired();
103103
if (!m_constructorOptions.inputDatasetAlias.empty())
104104
arg.AddAlias(m_constructorOptions.inputDatasetAlias);
105105
}
@@ -136,6 +136,20 @@ void GDALPipelineStepAlgorithm::AddRasterOutputArgs(bool hiddenForCLI)
136136
.SetMutualExclusionGroup(MUTUAL_EXCLUSION_GROUP_OVERWRITE_APPEND);
137137
}
138138

139+
/************************************************************************/
140+
/* GDALPipelineStepAlgorithm::AddVectorHiddenInputDatasetArg() */
141+
/************************************************************************/
142+
143+
void GDALPipelineStepAlgorithm::AddVectorHiddenInputDatasetArg()
144+
{
145+
AddInputDatasetArg(&m_inputDataset, GDAL_OF_VECTOR, false)
146+
.SetMinCount(0)
147+
.SetMaxCount(m_constructorOptions.inputDatasetMaxCount)
148+
.SetAutoOpenDataset(m_constructorOptions.autoOpenInputDatasets)
149+
.SetMetaVar(m_constructorOptions.inputDatasetMetaVar)
150+
.SetHidden();
151+
}
152+
139153
/************************************************************************/
140154
/* GDALPipelineStepAlgorithm::AddVectorInputArgs() */
141155
/************************************************************************/
@@ -147,12 +161,15 @@ void GDALPipelineStepAlgorithm::AddVectorInputArgs(bool hiddenForCLI)
147161
.SetHiddenForCLI(hiddenForCLI);
148162
AddOpenOptionsArg(&m_openOptions).SetHiddenForCLI(hiddenForCLI);
149163
auto &datasetArg =
150-
AddInputDatasetArg(&m_inputDataset, GDAL_OF_VECTOR,
151-
/* positionalAndRequired = */ !hiddenForCLI)
152-
.SetMinCount(1)
164+
AddInputDatasetArg(&m_inputDataset, GDAL_OF_VECTOR, false)
165+
.SetMinCount(m_constructorOptions.inputDatasetRequired ? 1 : 0)
153166
.SetMaxCount(m_constructorOptions.inputDatasetMaxCount)
154167
.SetAutoOpenDataset(m_constructorOptions.autoOpenInputDatasets)
155168
.SetHiddenForCLI(hiddenForCLI);
169+
if (m_constructorOptions.inputDatasetPositional && !hiddenForCLI)
170+
datasetArg.SetPositional();
171+
if (m_constructorOptions.inputDatasetRequired && !hiddenForCLI)
172+
datasetArg.SetRequired();
156173
if (m_constructorOptions.addInputLayerNameArgument)
157174
{
158175
auto &layerArg = AddArg(GDAL_ARG_NAME_INPUT_LAYER, 'l',
@@ -363,10 +380,13 @@ bool GDALPipelineStepAlgorithm::RunImpl(GDALProgressFunc pfnProgress,
363380
else
364381
{
365382
writeAlg->m_outputVRTCompatible = m_outputVRTCompatible;
366-
writeAlg->m_inputDataset.clear();
367-
writeAlg->m_inputDataset.resize(1);
368-
writeAlg->m_inputDataset[0].Set(
369-
m_outputDataset.GetDatasetRef());
383+
384+
std::vector<GDALArgDatasetValue> inputDataset(1);
385+
inputDataset[0].Set(m_outputDataset.GetDatasetRef());
386+
auto inputArg = writeAlg->GetArg(GDAL_ARG_NAME_INPUT);
387+
CPLAssert(inputArg);
388+
inputArg->Set(std::move(inputDataset));
389+
370390
if (pfnProgress)
371391
{
372392
pScaledData.reset(GDALCreateScaledProgress(

‎apps/gdalalg_raster_as_features.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ GDALRasterAsFeaturesAlgorithm::GDALRasterAsFeaturesAlgorithm(
4747
}
4848
else
4949
{
50+
AddRasterHiddenInputDatasetArg();
5051
AddOutputLayerNameArg(/* hiddenForCLI = */ false,
5152
/* shortNameOutputLayerAllowed = */ false);
5253
}

0 commit comments

Comments
 (0)