From 517de0a1e4d0c8ff60e90f8b6501a1fa49df20eb Mon Sep 17 00:00:00 2001 From: Ruslan Iushchenko Date: Tue, 1 Sep 2026 11:57:07 +0200 Subject: [PATCH 1/6] #871 Pass the full segment ID redefine map (instead of a list of redefines) to the copybook parser and store allowed segment ID values in `Group.segmentRedefineAllowedValues`; update tests accordingly. --- .../cobrix/cobol/parser/CopybookParser.scala | 16 ++--- .../cobol/parser/antlr/ParserVisitor.scala | 1 + .../absa/cobrix/cobol/parser/ast/Group.scala | 40 +++++++------ .../asttransform/RuleExpressionSetter.scala | 2 +- .../asttransform/SegmentRedefinesMarker.scala | 21 ++++--- .../cobol/reader/schema/CobolSchema.scala | 6 +- .../parse/ParentSegmentFieldsSpec.scala | 57 ++++++++++++------ .../cobol/parser/parse/ParserUtilsSpec.scala | 19 +++--- .../parser/parse/SegmentRedefinesSpec.scala | 60 ++++++++++++++----- .../cobol/CobolSchemaHierarchicalSpec.scala | 17 ++++-- 10 files changed, 156 insertions(+), 83 deletions(-) diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala index 9a40af567..432c3d4b9 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala @@ -112,7 +112,7 @@ object CopybookParser extends Logging { * @param dropGroupFillers Drop groups marked as fillers from the output AST. * @param dropValueFillers Drop primitive fields marked as fillers from the output AST. * @param fillerNamingPolicy Specifies a naming policy for fillers. - * @param segmentRedefines A list of redefined fields that correspond to various segments. This needs to be specified for automatically + * @param segmentIdRedefineMap A map from segment id values to corresponding field names * resolving segment redefines. * @param fieldParentMap A segment fields parent mapping. * @param stringTrimmingPolicy Specifies if and how strings should be trimmed when parsed. @@ -135,7 +135,7 @@ object CopybookParser extends Logging { dropGroupFillers: Boolean = false, dropValueFillers: Boolean = true, fillerNamingPolicy: FillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, - segmentRedefines: Seq[String] = Nil, + segmentIdRedefineMap: Map[String, String] = Map.empty, fieldParentMap: Map[String, String] = HashMap[String, String](), stringTrimmingPolicy: StringTrimmingPolicy = StringTrimmingPolicy.TrimBoth, isDisplayAlwaysString: Boolean = false, @@ -159,7 +159,7 @@ object CopybookParser extends Logging { dropGroupFillers, dropValueFillers, fillerNamingPolicy, - segmentRedefines, + segmentIdRedefineMap, fieldParentMap, stringTrimmingPolicy, isDisplayAlwaysString, @@ -208,7 +208,7 @@ object CopybookParser extends Logging { dropGroupFillers: Boolean = false, dropValueFillers: Boolean = true, fillerNamingPolicy: FillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, - segmentRedefines: Seq[String] = Nil, + segmentIdRedefineMap: Map[String, String] = Map.empty, fieldParentMap: Map[String, String] = HashMap[String, String](), stringTrimmingPolicy: StringTrimmingPolicy = StringTrimmingPolicy.TrimBoth, isDisplayAlwaysString: Boolean = false, @@ -232,7 +232,7 @@ object CopybookParser extends Logging { dropGroupFillers, dropValueFillers, fillerNamingPolicy, - segmentRedefines, + segmentIdRedefineMap, fieldParentMap, stringTrimmingPolicy, isDisplayAlwaysString, @@ -261,7 +261,7 @@ object CopybookParser extends Logging { * @param dropGroupFillers Drop groups marked as fillers from the output AST * @param dropValueFillers Drop primitive fields marked as fillers from the output AST * @param fillerNamingPolicy Specifies a naming policy for fillers - * @param segmentRedefines A list of redefined fields that correspond to various segments. This needs to be specified for automatically + * @param segmentIdRedefineMap A map from segment id values to corresponding field names * resolving segment redefines. * @param fieldParentMap A segment fields parent mapping * @param stringTrimmingPolicy Specifies if and how strings should be trimmed when parsed @@ -283,7 +283,7 @@ object CopybookParser extends Logging { dropGroupFillers: Boolean, dropValueFillers: Boolean, fillerNamingPolicy: FillerNamingPolicy, - segmentRedefines: Seq[String], + segmentIdRedefineMap: Map[String, String], fieldParentMap: Map[String, String], stringTrimmingPolicy: StringTrimmingPolicy, isDisplayAlwaysString: Boolean, @@ -324,7 +324,7 @@ object CopybookParser extends Logging { // Renames FILLERs that will be kept in the ast GroupFillersRenamer(dropGroupFillers, dropValueFillers, fillerNamingPolicy), // Sets isSegmentRedefine property of redefined groups - SegmentRedefinesMarker(segmentRedefines), + SegmentRedefinesMarker(segmentIdRedefineMap), // Sets parent groups for child segment redefines. SegmentParentsSetter(correctedFieldParentMap), // Add debugging fields if debug mode is enabled. diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/ParserVisitor.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/ParserVisitor.scala index 717a47105..70e2caffb 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/ParserVisitor.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/ParserVisitor.scala @@ -535,6 +535,7 @@ class ParserVisitor(enc: Encoding, redefines, isRedefined = false, isSegmentRedefine = false, + segmentRedefineAllowedValues = Nil, parentSegment = None, if (occurs.isDefined) Some(occurs.get.m) else None, if (occurs.isDefined) occurs.get.M else None, diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala index 29ce3a3b4..ae7ee9fa3 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala @@ -23,23 +23,24 @@ import scala.collection.mutable /** An abstraction for the non-leaves in the Cobol copybook * - * @param level A level for the statement - * @param name An identifier - * @param originalName Original name of the AST element (before the conversion to the Spark-compatible name) - * @param lineNumber An line number in the copybook - * @param children Child entities - * @param redefines A name of a field which is redefined by this one - * @param isRedefined Is the field redefined by an other field - * @param isSegmentRedefine Is the field corresponds to one of the segments (it should be a redefine) - * @param parentSegment Specifies a parent segment for a segment redefine in hierarchical files - * @param occurs The number of elements in an fixed size array / minimum items in variable-sized array - * @param to The maximum number of items in a variable size array - * @param dependingOn A field which specifies size of the array in a record - * @param isFiller Is the group a filler (unnamed block of data) - * @param groupUsage A USAGE to be inherited by all the fields in the group - * @param nonFillerSize The number of non-filler children in the group - * @param binaryProperties Pre-calculated offsets and sizes of thebinary data of the group - * @param parent A parent node + * @param level A level for the statement + * @param name An identifier + * @param originalName Original name of the AST element (before the conversion to the Spark-compatible name) + * @param lineNumber An line number in the copybook + * @param children Child entities + * @param redefines A name of a field which is redefined by this one + * @param isRedefined Is the field redefined by an other field + * @param isSegmentRedefine Is the field corresponds to one of the segments (it should be a redefine) + * @param segmentRedefineAllowedValues The list of values for the SEGMENT_ID field if the field is a segment redefine (isSegmentRedefine = true) + * @param parentSegment Specifies a parent segment for a segment redefine in hierarchical files + * @param occurs The number of elements in an fixed size array / minimum items in variable-sized array + * @param to The maximum number of items in a variable size array + * @param dependingOn A field which specifies size of the array in a record + * @param isFiller Is the group a filler (unnamed block of data) + * @param groupUsage A USAGE to be inherited by all the fields in the group + * @param nonFillerSize The number of non-filler children in the group + * @param binaryProperties Pre-calculated offsets and sizes of thebinary data of the group + * @param parent A parent node */ case class Group( level: Int, @@ -50,6 +51,7 @@ case class Group( redefines: Option[String] = None, isRedefined: Boolean = false, isSegmentRedefine: Boolean = false, + segmentRedefineAllowedValues: Seq[String] = Nil, parentSegment: Option[Group] = None, occurs: Option[Int] = None, to: Option[Int] = None, @@ -108,6 +110,10 @@ case class Group( copy(isSegmentRedefine = newIsSegmentRedefine)(parent) } + def withUpdatedSegmentRedefineValues(newSegmentRedefineAllowedValues: Seq[String]): Group = { + copy(segmentRedefineAllowedValues = newSegmentRedefineAllowedValues)(parent) + } + /** Returns the original AST element with updated `isSegmentRedefine` flag */ def withUpdatedParentSegment(newParentSegmentOpt: Option[Group]): Group = { copy(parentSegment = newParentSegmentOpt)(parent) diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/RuleExpressionSetter.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/RuleExpressionSetter.scala index 8c74f98db..e307734d6 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/RuleExpressionSetter.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/RuleExpressionSetter.scala @@ -30,7 +30,7 @@ class RuleExpressionSetter( private val log = LoggerFactory.getLogger(this.getClass) /** - * Sets isDependee attribute for fields in the schema which are used by other fields in DEPENDING ON clause + * Sets newIsUsedInRules attribute for fields in the schema which are used by other fields in redefine rule expressions * * @param ast An AST as a set of copybook records * @return The same AST with binary properties set for every field diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala index c31b7e64d..bf27e2db9 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala @@ -17,16 +17,16 @@ package za.co.absa.cobrix.cobol.parser.asttransform import za.co.absa.cobrix.cobol.parser.CopybookParser -import za.co.absa.cobrix.cobol.parser.CopybookParser.CopybookAST +import za.co.absa.cobrix.cobol.parser.CopybookParser.{CopybookAST, transformIdentifier} import za.co.absa.cobrix.cobol.parser.ast.{Group, Primitive, Statement} import scala.collection.mutable import scala.collection.mutable.ArrayBuffer /** - * @param segmentRedefines The list of fields names that correspond to segment GROUPs. + * @param segmentIdRedefineMap The map from segment ID values to group names. Key = a segment id, Value = a redefined field */ -class SegmentRedefinesMarker(segmentRedefines: Seq[String]) extends AstTransformer { +class SegmentRedefinesMarker(segmentIdRedefineMap: Map[String, String]) extends AstTransformer { /** * Sets isSegmentRedefine property of redefined groups so the row extractor be able to skip parsing segment groups * that do not belong to a particular segment id. @@ -42,8 +42,8 @@ class SegmentRedefinesMarker(segmentRedefines: Seq[String]) extends AstTransform */ final override def transform(ast: CopybookAST): CopybookAST = { val foundRedefines = new mutable.HashSet[String] - val transformedSegmentRedefines = segmentRedefines.map(CopybookParser.transformIdentifier) - val allowNonRedefines = segmentRedefines.lengthCompare(1) == 0 + val transformedSegmentRedefines = segmentIdRedefineMap.values.toSeq.distinct.map(CopybookParser.transformIdentifier) + val allowNonRedefines = transformedSegmentRedefines.lengthCompare(1) == 0 var redefineGroupState = 0 def ensureSegmentRedefinesAreIneGroup(currentField: String, isCurrentFieldASegmentRedefine: Boolean): Unit = { @@ -76,9 +76,11 @@ class SegmentRedefinesMarker(segmentRedefines: Seq[String]) extends AstTransform if (redefineGroupState == 1 && g.redefines.isEmpty) throw new IllegalStateException(s"The segment redefine field '${g.name}' is not a REDEFINE or redefined by another field.") + val allowedValues = segmentIdRedefineMap.filter(_._2.equalsIgnoreCase(g.name)).keys.toSeq.distinct ensureSegmentRedefinesAreIneGroup(g.name, isCurrentFieldASegmentRedefine = true) foundRedefines += g.name g.withUpdatedIsSegmentRedefine(true) + .withUpdatedSegmentRedefineValues(allowedValues) } else { // Allow redefines in between segment redefines. val fieldMightBeRedefine = if (redefineGroupState == 1 && g.redefines.nonEmpty) @@ -117,7 +119,7 @@ class SegmentRedefinesMarker(segmentRedefines: Seq[String]) extends AstTransform } } - if (segmentRedefines.isEmpty) { + if (segmentIdRedefineMap.isEmpty) { ast } else { val isFlatAst = ast.children.exists(_.isInstanceOf[Primitive]) @@ -133,5 +135,10 @@ class SegmentRedefinesMarker(segmentRedefines: Seq[String]) extends AstTransform } object SegmentRedefinesMarker { - def apply(segmentRedefines: Seq[String]): SegmentRedefinesMarker = new SegmentRedefinesMarker(segmentRedefines) + def apply(segmentIdRedefineMap: Map[String, String]): SegmentRedefinesMarker = { + val transformerMap = segmentIdRedefineMap.map { + case (k, v) => (k, transformIdentifier(v)) + } + new SegmentRedefinesMarker(transformerMap) + } } diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala index 0359ad701..c29bc74fa 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala @@ -91,7 +91,7 @@ object CobolSchema { } val encoding = if (readerParameters.isEbcdic) EBCDIC else ASCII - val segmentRedefines = readerParameters.multisegment.map(r => r.segmentIdRedefineMap.values.toList.distinct).getOrElse(Nil) + val segmentIdRedefineMap = readerParameters.multisegment.map(r => r.segmentIdRedefineMap).getOrElse(Map.empty[String, String]) val fieldParentMap = readerParameters.multisegment.map(r => r.fieldParentMap).getOrElse(HashMap[String, String]()) val codePage = getCodePage(readerParameters.ebcdicCodePage, readerParameters.ebcdicCodePageClass) val asciiCharset = readerParameters.asciiCharset match { @@ -107,7 +107,7 @@ object CobolSchema { readerParameters.dropGroupFillers, readerParameters.dropValueFillers, readerParameters.fillerNamingPolicy, - segmentRedefines, + segmentIdRedefineMap, fieldParentMap, readerParameters.stringTrimmingPolicy, readerParameters.isDisplayAlwaysString, @@ -133,7 +133,7 @@ object CobolSchema { readerParameters.dropGroupFillers, readerParameters.dropValueFillers, readerParameters.fillerNamingPolicy, - segmentRedefines, + segmentIdRedefineMap, fieldParentMap, readerParameters.stringTrimmingPolicy, readerParameters.isDisplayAlwaysString, diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParentSegmentFieldsSpec.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParentSegmentFieldsSpec.scala index 123d711e1..0ceed44d4 100644 --- a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParentSegmentFieldsSpec.scala +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParentSegmentFieldsSpec.scala @@ -16,12 +16,12 @@ package za.co.absa.cobrix.cobol.parser.parse -import java.io.{ByteArrayOutputStream, ObjectOutputStream} import org.scalatest.wordspec.AnyWordSpec import za.co.absa.cobrix.cobol.parser.CopybookParser import za.co.absa.cobrix.cobol.parser.ast.Group import za.co.absa.cobrix.cobol.parser.policies.FillerNamingPolicy +import java.io.{ByteArrayOutputStream, ObjectOutputStream} import scala.collection.immutable.HashMap class ParentSegmentFieldsSpec extends AnyWordSpec { @@ -35,18 +35,18 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { | 03 FIELD2 PIC X(2). """.stripMargin - val segmentRedefines: Seq[String] = Nil + val segmentIdRedefineMap: Map[String, String] = Map.empty val fieldParentMap = HashMap[String, String]() "CopybookParser.parseTree" should { "not throw if no segment redefines or parent fields are provided" in { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) } } "CopybookParser.getParentToChildrenMap" should { "return an empty map" in { - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val map = CopybookParser.getParentToChildrenMap(parsedCopybook.ast) assert(map.isEmpty) @@ -65,23 +65,29 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { | 03 FIELD3 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: Nil + val segmentRedefinesMap = Map ( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B" + ) val fieldParentMap = HashMap[String, String]("SEGMENT-B" -> "SEGMENT-A") "CopybookParser.parseTree" should { "work with a simple 2 segments having a parent-child relationship" in { - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesMap, fieldParentMap) assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(0).asInstanceOf[Group].parentSegment.isEmpty) assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(1).asInstanceOf[Group].parentSegment.nonEmpty) assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(2).asInstanceOf[Group].parentSegment.isEmpty) + assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(0).asInstanceOf[Group].segmentRedefineAllowedValues.contains("A")) + assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(1).asInstanceOf[Group].segmentRedefineAllowedValues.contains("B")) + assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(2).asInstanceOf[Group].segmentRedefineAllowedValues.isEmpty) } } "CopybookParser.getParentToChildrenMap" should { "return a single entity map" in { - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesMap, fieldParentMap) val map = CopybookParser.getParentToChildrenMap(parsedCopybook.ast) @@ -111,13 +117,18 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { | 03 FIELD-6 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-C" :: "SEGMENT-B" :: Nil + val segmentIdRedefineMap = Map ( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C" + ) + val fieldParentMap = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "SEGMENT-A") "CopybookParser.getParentToChildrenMap" should { "return a proper parent-children map" in { - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val map = CopybookParser.getParentToChildrenMap(parsedCopybook.ast) @@ -153,10 +164,14 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { | 03 FIELD4 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: "SEGMENT-C" :: Nil + val segmentIdRedefineMap = Map ( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C" + ) val fieldParentMap = HashMap[String, String]("SEGMENT-B" -> "SEGMENT-A", "SEGMENT-C" -> "SEGMENT-B") - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val bos = new ByteArrayOutputStream val out = new ObjectOutputStream(bos) @@ -183,19 +198,23 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { | 03 FIELD-6 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-C" :: "SEGMENT-B" :: Nil + val segmentIdRedefineMap = Map ( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C" + ) "a correct mapping is specified, should no throw" in { val fieldParentMapOk = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "SEGMENT-A") - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapOk) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapOk) } "a one of the mapped fields is not a segment redefine, should throw an exception" in { val fieldParentMapSegmentRedefine = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "SEGMENT-A", "SEGMENT-A" -> "SEGMENT-D") val ex = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapSegmentRedefine) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapSegmentRedefine) } assert(ex.getMessage.contains("Field SEGMENT_D is specified to be the parent of SEGMENT_A, but SEGMENT_D is not a segment redefine.")) } @@ -204,7 +223,7 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { val fieldParentMapTwoRoots = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A") val ex = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapTwoRoots) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapTwoRoots) } assert(ex.getMessage.contains("Only one root segment is allowed. Found root segments: [ SEGMENT_A, SEGMENT_B ]")) } @@ -213,7 +232,7 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { val fieldParentMapNonSegment = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "RECORD-1") val ex = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapNonSegment) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapNonSegment) } assert(ex.getMessage.contains("Field RECORD_1 is specified to be the parent of SEGMENT_B, but RECORD_1 is not a segment redefine")) } @@ -222,7 +241,7 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { val fieldParentMapSelfParent = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-C", "SEGMENT-B" -> "SEGMENT-B") val ex = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapSelfParent) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapSelfParent) } assert(ex.getMessage.contains("A segment SEGMENT_C cannot be a parent of itself") || ex.getMessage.contains("A segment SEGMENT_B cannot be a parent of itself")) } @@ -231,7 +250,7 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { val fieldParentMapCycle = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-B", "SEGMENT-B" -> "SEGMENT-C") val ex = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapCycle) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapCycle) } assert(ex.getMessage.contains("Segments parent-child relation form a cycle: SEGMENT_C, SEGMENT_B, SEGMENT_C") || ex.getMessage.contains("Segments parent-child relation form a cycle: SEGMENT_B, SEGMENT_C, SEGMENT_B")) @@ -241,7 +260,7 @@ class ParentSegmentFieldsSpec extends AnyWordSpec { val fieldParentMapNotExist = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "SEGMENT-Z") val ex = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMapNotExist) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMapNotExist) } assert(ex.getMessage.contains("Field SEGMENT_Z is specified to be the parent of SEGMENT_B, but SEGMENT_Z is not a segment redefine")) } diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParserUtilsSpec.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParserUtilsSpec.scala index 04d1f90d9..cc78ff3b0 100644 --- a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParserUtilsSpec.scala +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/ParserUtilsSpec.scala @@ -49,7 +49,12 @@ class ParserUtilsSpec extends AnyWordSpec { | 03 FIELD-6 PIC X(2). """.stripMargin - private val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: "SEGMENT-C" :: "SEGMENT-D" :: Nil + private val segmentIdRedefineMap = Map( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C", + "D" -> "SEGMENT-D" + ) private val fieldParentMap = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "SEGMENT-A", "SEGMENT-D" -> "SEGMENT-B") "CopybookParser.findCycleIntAMap" should { @@ -94,10 +99,10 @@ class ParserUtilsSpec extends AnyWordSpec { "CopybookParser.getAllSegmentRedefines" should { "return an empty list if no segment redefines are defined" in { - val segmentRedefines: Seq[String] = Nil + val segmentIdRedefineMap: Map[String, String] = Map.empty val fieldParentMap = HashMap[String, String]() - val parsedCopybook = CopybookParser.parseTree(simpleCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(simpleCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val redefines = CopybookParser.getAllSegmentRedefines(parsedCopybook.ast) @@ -105,7 +110,7 @@ class ParserUtilsSpec extends AnyWordSpec { } "return a list of segment redefines for a hierarchical copybook" in { - val parsedCopybook = CopybookParser.parseTree(hierarchicalCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(hierarchicalCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val redefines = CopybookParser.getAllSegmentRedefines(parsedCopybook.ast) @@ -115,10 +120,10 @@ class ParserUtilsSpec extends AnyWordSpec { "CopybookParser.getRootSegmentAST" should { "return the same AST if no parent segments are defined" in { - val segmentRedefines: Seq[String] = Nil + val segmentIdRedefineMap: Map[String, String] = Map.empty val fieldParentMap = HashMap[String, String]() - val parsedCopybook = CopybookParser.parseTree(simpleCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(simpleCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val rootAst = CopybookParser.getRootSegmentAST(parsedCopybook.ast) @@ -128,7 +133,7 @@ class ParserUtilsSpec extends AnyWordSpec { } "return an AST without parent segments for a hierarchical copybook" in { - val parsedCopybook = CopybookParser.parseTree(hierarchicalCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines, fieldParentMap) + val parsedCopybook = CopybookParser.parseTree(hierarchicalCopybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap, fieldParentMap) val rootAst = CopybookParser.getRootSegmentAST(parsedCopybook.ast) diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/SegmentRedefinesSpec.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/SegmentRedefinesSpec.scala index a15d56897..ea66ba9f6 100644 --- a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/SegmentRedefinesSpec.scala +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/SegmentRedefinesSpec.scala @@ -32,9 +32,9 @@ class SegmentRedefinesSpec extends AnyFunSuite { | 03 FIELD5 PIC X(2). """.stripMargin - val segmentRedefines: Seq[String] = Nil + val segmentIdRedefineMap: Map[String, String] = Map.empty - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap) } test ("Test segment redefines should worked if only one segment is specified") { @@ -46,9 +46,9 @@ class SegmentRedefinesSpec extends AnyFunSuite { | 03 FIELD5 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: Nil + val segmentIdRedefineMap = Map("A" -> "SEGMENT-A") - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMap) assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(0).asInstanceOf[Group].isSegmentRedefine) assert(!parsedCopybook.ast.children.head.asInstanceOf[Group].children(1).asInstanceOf[Group].isSegmentRedefine) @@ -69,14 +69,24 @@ class SegmentRedefinesSpec extends AnyFunSuite { | 03 FIELD5 PIC X(2). """.stripMargin - val segmentRedefinesOk = "SEGMENT-A" :: "SEGMENT-C" :: "SEGMENT-B" :: Nil - val segmentRedefinesMissing = "SEGMENT-A" :: "SEGMENT-C" :: "SEGMENT-B" :: "SEGMENT-D" :: Nil + val segmentIdRedefineMapOk = Map( + "A" -> "SEGMENT-A", + "C" -> "SEGMENT-C", + "B" -> "SEGMENT-B" + ) - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesOk) + val segmentIdRedefineMapMissing = Map( + "A" -> "SEGMENT-A", + "C" -> "SEGMENT-C", + "B" -> "SEGMENT-B", + "D" -> "SEGMENT-D" + ) + + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMapOk) // If a segment redefine is missing in the copybook an exception should be raised val exception1 = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesMissing) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMapMissing) } assert(exception1.getMessage.contains("The following segment redefines not found: [ SEGMENT_D ]")) @@ -105,9 +115,13 @@ class SegmentRedefinesSpec extends AnyFunSuite { | 03 FIELD5 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-C" :: "SEGMENT-D" :: Nil + val segmentIdRedefineMapOk = Map( + "A" -> "SEGMENT-A", + "C" -> "SEGMENT-C", + "D" -> "SEGMENT-D" + ) - val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines) + val parsedCopybook = CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentIdRedefineMapOk) assert(!parsedCopybook.ast.children.head.asInstanceOf[Group].children(0).asInstanceOf[Group].isSegmentRedefine) assert(parsedCopybook.ast.children.head.asInstanceOf[Group].children(1).asInstanceOf[Group].isSegmentRedefine) @@ -135,9 +149,15 @@ class SegmentRedefinesSpec extends AnyFunSuite { """.stripMargin val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: "SEGMENT-C" :: "SEGMENT-D" :: Nil + val segmentRedefinesMap = Map( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C", + "D" -> "SEGMENT-D" + ) val exception1 = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesMap) } assert(exception1.getMessage.contains("The 'SEGMENT_C' field is specified to be a segment redefine.")) } @@ -159,10 +179,15 @@ class SegmentRedefinesSpec extends AnyFunSuite { | 03 FIELD5 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: "SEGMENT-C" :: "SEGMENT-D" :: Nil + val segmentRedefinesMap = Map( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C", + "D" -> "SEGMENT-D" + ) val exception1 = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesMap) } assert(exception1.getMessage.contains("The following segment redefines not found: [ SEGMENT_A ].")) } @@ -184,10 +209,15 @@ class SegmentRedefinesSpec extends AnyFunSuite { | 03 FIELD5 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: "SEGMENT-C" :: "SEGMENT-D" :: Nil + val segmentRedefinesMap = Map( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C", + "D" -> "SEGMENT-D" + ) val exception1 = intercept[IllegalStateException] { - CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefines) + CopybookParser.parseTree(copybook, dropGroupFillers = false, dropValueFillers = true, fillerNamingPolicy = FillerNamingPolicy.SequenceNumbers, segmentRedefinesMap) } assert(exception1.getMessage.contains("The segment redefine field 'SEGMENT_C' is not a REDEFINE or redefined by another field.")) } diff --git a/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/CobolSchemaHierarchicalSpec.scala b/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/CobolSchemaHierarchicalSpec.scala index 0d530bfb2..1167d2159 100644 --- a/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/CobolSchemaHierarchicalSpec.scala +++ b/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/CobolSchemaHierarchicalSpec.scala @@ -46,10 +46,10 @@ class CobolSchemaHierarchicalSpec extends AnyWordSpec { | | |-- FIELD3: string (nullable = true) |""".stripMargin.replace("\r\n", "\n") - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: Nil + val segmentIdRedefineMap = Map("A" -> "SEGMENT-A", "B" -> "SEGMENT-B") val fieldParentMap = HashMap[String, String]("SEGMENT-B" -> "SEGMENT-A") - val cobolSchema = parseSchema(copybook, segmentRedefines, fieldParentMap) + val cobolSchema = parseSchema(copybook, segmentIdRedefineMap, fieldParentMap) assert(cobolSchema.getSparkSchema.treeString == expectedSchema) } @@ -72,7 +72,12 @@ class CobolSchemaHierarchicalSpec extends AnyWordSpec { | 03 FIELD-6 PIC X(2). """.stripMargin - val segmentRedefines = "SEGMENT-A" :: "SEGMENT-B" :: "SEGMENT-C" :: "SEGMENT-D" :: Nil + val segmentIdRedefineMap = Map( + "A" -> "SEGMENT-A", + "B" -> "SEGMENT-B", + "C" -> "SEGMENT-C", + "D" -> "SEGMENT-D" + ) val fieldParentMap = HashMap[String, String]("SEGMENT-C" -> "SEGMENT-A", "SEGMENT-B" -> "SEGMENT-A", "SEGMENT-D" -> "SEGMENT-C") val expectedSchema = @@ -94,13 +99,13 @@ class CobolSchemaHierarchicalSpec extends AnyWordSpec { | | |-- FIELD_6: string (nullable = true) |""".stripMargin.replace("\r\n", "\n") - val cobolSchema = parseSchema(copybook, segmentRedefines, fieldParentMap) + val cobolSchema = parseSchema(copybook, segmentIdRedefineMap, fieldParentMap) assert(cobolSchema.getSparkSchema.treeString == expectedSchema) } - private def parseSchema(copybook: String, segmentRedefines: List[String], fieldParentMap: Map[String, String]): CobolSchema = { - val parsedSchema = CopybookParser.parseTree(copybook, segmentRedefines = segmentRedefines, fieldParentMap = fieldParentMap) + private def parseSchema(copybook: String, segmentIdRedefineMap: Map[String, String], fieldParentMap: Map[String, String]): CobolSchema = { + val parsedSchema = CopybookParser.parseTree(copybook, segmentIdRedefineMap = segmentIdRedefineMap, fieldParentMap = fieldParentMap) CobolSchema.builder(parsedSchema).build() } } From 8d33929c29edb80e7a02323f875996e5d74395c8 Mon Sep 17 00:00:00 2001 From: Ruslan Iushchenko Date: Thu, 3 Sep 2026 09:06:45 +0200 Subject: [PATCH 2/6] #871 Add `ParentGroupSetter` AST transformer to fix stale `parent` references after AST transformations This required making `parent` mutable in `Group` and `Primitive`. --- .../absa/cobrix/cobol/parser/Copybook.scala | 3 +- .../cobrix/cobol/parser/CopybookParser.scala | 11 +++- .../absa/cobrix/cobol/parser/ast/Group.scala | 2 +- .../cobrix/cobol/parser/ast/Primitive.scala | 2 +- .../asttransform/ParentGroupSetter.scala | 58 +++++++++++++++++++ 5 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/ParentGroupSetter.scala diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala index dc2e13941..05b89b119 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala @@ -411,7 +411,8 @@ object Copybook { } // recompute sizes - val schema = BinaryPropertiesAdder().transform(newRoot) + val schema1 = BinaryPropertiesAdder().transform(newRoot) + val schema = ParentGroupSetter().transform(schema1) new Copybook(schema) } diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala index 432c3d4b9..1d4905503 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala @@ -332,14 +332,21 @@ object CopybookParser extends Logging { // For each group calculates the number of non-filler items. NonFillerCountSetter(), // Sets isUsedInRules and rule expressions for each field - RuleExpressionSetter(redefineRuleExpressions) + RuleExpressionSetter(redefineRuleExpressions), + // Updates 'parent' field of each of the fields in case they are inconsistent + ParentGroupSetter() ) val transformedAst = transformers.foldLeft(schemaANTLR) { (ast, transformer) => transformer.transform(ast) } - new Copybook(transformedAst) + val finalAst = if (transformers.nonEmpty) + ParentGroupSetter().transform(transformedAst) + else + transformedAst + + new Copybook(finalAst) } /** diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala index ae7ee9fa3..aee18c6bf 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Group.scala @@ -63,7 +63,7 @@ case class Group( ruleExpression: Option[ExpressionEvaluator] = None, binaryProperties: BinaryProperties = BinaryProperties(0, 0, 0) ) - (val parent: Option[Group] = None) + (var parent: Option[Group] = None) extends Statement { /** This method is used to add a [[za.co.absa.cobrix.cobol.parser.ast.Statement]] object as a child of diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Primitive.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Primitive.scala index c67f8d7e1..a1beff9a2 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Primitive.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/ast/Primitive.scala @@ -61,7 +61,7 @@ case class Primitive( encode: Option[EncoderSelector.Encoder], binaryProperties: BinaryProperties = BinaryProperties(0, 0, 0) ) - (val parent: Option[Group] = None) + (var parent: Option[Group] = None) extends Statement { /** This is cached value specifying if the field is a string */ diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/ParentGroupSetter.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/ParentGroupSetter.scala new file mode 100644 index 000000000..a7fda652a --- /dev/null +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/ParentGroupSetter.scala @@ -0,0 +1,58 @@ +/* + * Copyright 2018 ABSA Group Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package za.co.absa.cobrix.cobol.parser.asttransform + +import za.co.absa.cobrix.cobol.parser.CopybookParser.CopybookAST +import za.co.absa.cobrix.cobol.parser.ast.{Group, Primitive} + +/** + * An AST transformer that restores the parent references of every element of a copybook AST. + * + * The AST is traversed recursively from the root group down to the leaves, and each group and primitive + * field is recreated so that its `parent` points to the newly created copy of its enclosing group. The root + * of the AST is left without a parent. + * + * This transformation is required because AST elements are immutable case classes: whenever a copybook AST is + * rebuilt or modified by other transformations, the parent links of the affected elements may point to stale + * copies of their parents. Applying this transformer as the final step ensures that the parent links are + * consistent with the actual hierarchy of the resulting AST. + * + * The structure, ordering and all other properties of the fields remain unchanged. + */ +class ParentGroupSetter extends AstTransformer { + final override def transform(ast: CopybookAST): CopybookAST = { + def processGroup(group: Group, parent: Option[Group]): Unit = { + var i = 0 + group.parent = parent + while (i < group.children.length) { + group.children(i) match { + case g: Group => processGroup(g, Some(group)) + case p: Primitive => p.parent = Some(group) + } + i += 1 + } + } + + processGroup(ast, None) + ast + } +} + + +object ParentGroupSetter { + def apply(): ParentGroupSetter = new ParentGroupSetter() +} From e343b3608a70f4c47254191bfb5b56b153e3f7ad Mon Sep 17 00:00:00 2001 From: Ruslan Iushchenko Date: Fri, 4 Sep 2026 09:16:06 +0200 Subject: [PATCH 3/6] #871 Add `Copybook` helpers for evaluating REDEFINE rules per record: `hasRedefineRules`, `extractExpressionVariablesFromRecord`, `isFieldEnabled` and `isPartOfSegment`. Also propagate the variable size OCCURS policy from reader parameters to `Copybook` and preserve it across copybook transformations. --- .../absa/cobrix/cobol/parser/Copybook.scala | 225 +++++++++++++++- .../cobrix/cobol/parser/CopybookParser.scala | 13 +- .../cobol/reader/schema/CobolSchema.scala | 2 + .../cobrix/cobol/parser/CopybookSuite.scala | 247 ++++++++++++++++++ .../cobol/SparkCobolProcessorSuite.scala | 172 +++++++++++- 5 files changed, 651 insertions(+), 8 deletions(-) create mode 100644 cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala index 05b89b119..452ffda43 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala @@ -20,10 +20,14 @@ import za.co.absa.cobrix.cobol.internal.Logging import za.co.absa.cobrix.cobol.parser.CopybookParser.CopybookAST import za.co.absa.cobrix.cobol.parser.ast.datatype.{AlphaNumeric, COMP3, Decimal, Integral} import za.co.absa.cobrix.cobol.parser.ast.{Group, Primitive, Statement} -import za.co.absa.cobrix.cobol.parser.asttransform.BinaryPropertiesAdder +import za.co.absa.cobrix.cobol.parser.asttransform.{BinaryPropertiesAdder, ParentGroupSetter} +import za.co.absa.cobrix.cobol.parser.policies.VariableSizeOccursPolicy +import za.co.absa.cobrix.cobol.reader.extractors.record.RecordExtractors +import za.co.absa.cobrix.cobol.reader.extractors.record.RecordExtractors.canExtract import za.co.absa.cobrix.cobol.reader.parameters.WriterParameters import java.util.concurrent.ConcurrentHashMap +import scala.annotation.tailrec import scala.collection.mutable import scala.collection.mutable.ArrayBuffer @@ -33,6 +37,7 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { private val cachePrimitives = new ConcurrentHashMap[String, Primitive]() private val cacheStatements = new ConcurrentHashMap[String, Statement]() + private[cobrix] var variableSizeOccursPolicy: VariableSizeOccursPolicy = VariableSizeOccursPolicy.MaxSize val isFlatCopybook: Boolean = ast.children.exists(f => f.isInstanceOf[Primitive]) @@ -78,6 +83,203 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { } } + lazy val hasRedefineRules: Boolean = { + def hasAnyRules(group: Group): Boolean = { + group.children.exists { + case g: Group => g.ruleExpression.nonEmpty || hasAnyRules(g) + case p: Primitive => p.isUsedInRules || p.ruleExpression.nonEmpty + } + } + hasAnyRules(ast) + } + + /** + * Traverses the copybook AST over the given record and collects the values of all fields that + * participate in expression evaluation, such as fields referenced by conditional expressions + * ("is used in rules") and fields used as `DEPENDING ON` counters of variable size arrays. + * + * Fields are decoded lazily: groups and primitives that cannot be extracted for the given record, + * as well as segment redefines that do not match the provided segment id, are skipped while the + * offset is advanced according to their binary size. + * + * @param recordBytes The raw bytes of the record to extract the variables from. + * @param segmentIdValue The value of the segment id of the record, if the copybook contains + * segment redefines. Redefined groups that do not allow this segment id + * are not decoded. + * @param startOffset The offset, in bits, at which the root record starts inside the given bytes. + * @return A mutable map from field names to their decoded values that can be used as the variable + * context for expression evaluation. + */ + def extractExpressionVariablesFromRecord(recordBytes: Array[Byte], + segmentIdValue: Option[String] = None, + startOffset: Int = 0): mutable.HashMap[String, Any] = { + if (!hasRedefineRules) return mutable.HashMap.empty[String, Any] + + val dependFields = scala.collection.mutable.HashMap.empty[String, Either[Int, String]] + val variables = new mutable.HashMap[String, Any]() + + def skipArray(field: Statement): Int = { + val arraySize = field.arrayMaxSize + val actualSize = field.dependingOn match { + case None => arraySize + case Some(dependingOn) => + val dependValue: Int = dependFields.getOrElse(dependingOn, Left(arraySize)) match { + case Left(n) => n + case Right(s) => field.dependingOnHandlers.getOrElse(s, arraySize) + } + if (dependValue >= field.arrayMinSize && dependValue <= arraySize) + dependValue + else + arraySize + } + + variableSizeOccursPolicy match { + case VariableSizeOccursPolicy.MaxSize => + field.binaryProperties.actualSize + case VariableSizeOccursPolicy.ShiftRecord => + (field.binaryProperties.actualSize / arraySize) * actualSize + case VariableSizeOccursPolicy.PadRecord => + (field.binaryProperties.actualSize / arraySize) * actualSize + } + } + + def processValue(field: Statement, offset: Int): Int = { + field match { + case grp: Group => + if (grp.isSegmentRedefine && segmentIdValue.nonEmpty && !grp.segmentRedefineAllowedValues.contains(segmentIdValue.get)) { + grp.binaryProperties.actualSize + } else { + val extract = canExtract(grp, variables) + if (extract) { + processGroup(grp, offset) + } else { + grp.binaryProperties.actualSize + } + } + case st: Primitive => + val extract = canExtract(st, variables) + if (extract && (st.isUsedInRules || st.isDependee)) { + val value = st.decodeTypeValue(offset, recordBytes) + if (st.isUsedInRules) { + variables += st.name -> value + } + if (value != null && st.isDependee) { + val intStringVal: Either[Int, String] = value match { + case v: Int => Left(v) + case v: Number => Left(v.intValue()) + case v: String => Right(v) + case v => throw new IllegalStateException(s"Field ${st.name} is an a DEPENDING ON field of an OCCURS, should be integral or 'occurs_mapping' should be defined, found ${v.getClass}.") + } + dependFields += st.name -> intStringVal + } + st.binaryProperties.actualSize + } else { + st.binaryProperties.actualSize + } + } + } + + def processGroup(group: Group, offset: Int): Int = { + var bitOffset = offset + var j = 0 + var i = 0 + while (i < group.children.length) { + val field = group.children(i) + if (field.isArray) { + val size = skipArray(field) + if (!field.isRedefined) { + bitOffset += size + } + } else { + val size = processValue(field, bitOffset) + if (!field.isRedefined) { + if (field.redefines.isDefined) { + bitOffset += field.binaryProperties.actualSize + } else { + bitOffset += size + } + } + } + if (!field.isFiller) { + j += 1 + } + i += 1 + } + bitOffset - offset + } + + processGroup(ast, startOffset) + variables + } + + /** + * Determines whether a given field should be processed for the current record. + * + * A field is considered enabled when both of the following conditions hold: + * it belongs to the segment identified by the provided segment id value (when a segment id value + * is available; if no segment id value is provided, the segment check is skipped and the field is + * treated as belonging to the current segment), and it can be extracted according to the values of + * expression variables (defined by REDEFINE rules) collected from the record. + * + * Use `extractExpressionVariablesFromRecord` to obtain the values of expression variables for the current record. + * + * @param field A field (AST statement) of the copybook to check. + * @param segmentIdValueOpt An optional value of the segment id of the current record. + * @param recordVariables A map of expression variable names to their values extracted from the current record. + * @return true if the field is part of the current segment and can be extracted from the record. + */ + def isFieldEnabled(field: Statement, segmentIdValueOpt: Option[String], recordVariables: mutable.HashMap[String, Any]): Boolean = { + val isCorrectSegment = segmentIdValueOpt match { + case Some(segmentIdValue) => + isPartOfSegment(field, segmentIdValue) + case None => true + } + + if (isCorrectSegment) { + RecordExtractors.canExtract(field, recordVariables) + } else { + false + } + } + + /** + * Determines whether a given field belongs to a segment identified by the specified segment id value. + * + * The method walks up the AST from the field's parent looking for the closest enclosing group that is + * a segment redefine. If such a group is found, the field is considered part of the segment only when + * the group's allowed segment id values contain the given segment id value. If the field is not + * located inside any segment redefine (or has no parent at all), it is considered to be part of + * every segment. + * + * @param field A field (AST statement) of the copybook to check. + * @param segmentIdValue A value of the segment id of the current record. + * @return true if the field belongs to the segment corresponding to the given segment id value. + */ + def isPartOfSegment(field: Statement, segmentIdValue: String): Boolean = { + @tailrec + def getSegmentRedefineGroup(g: Group): Option[Group] = { + if (g.isSegmentRedefine) { + Some(g) + } else{ + g.parent match { + case Some(parent) => getSegmentRedefineGroup(parent) + case None => None + } + } + } + + field.parent match { + case Some(p) => + getSegmentRedefineGroup(p) match { + case Some(segmentRedefine) => + segmentRedefine.segmentRedefineAllowedValues.contains(segmentIdValue) + case None => + true + } + case None => true + } + } + /** * Get value of a field of the copybook record by name * @@ -279,7 +481,9 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { throw new RuntimeException("All elements of the root element must be record groups.") val newRoot = ast.children.head.asInstanceOf[Group].copy()(None) - new Copybook(BinaryPropertiesAdder().transform(newRoot)) + val cpy = new Copybook(BinaryPropertiesAdder().transform(newRoot)) + cpy.setVariableSizeOccursPolicy(variableSizeOccursPolicy) + cpy } def dropFillers(dropGroupFillers: Boolean, dropValueFillers: Boolean): Copybook = { @@ -305,7 +509,10 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { } dropFillersAst(ast) match { - case Some(newAst) => new Copybook(newAst) + case Some(newAst) => + val cpy = new Copybook(newAst) + cpy.setVariableSizeOccursPolicy(variableSizeOccursPolicy) + cpy case None => throw new IllegalArgumentException("Removing of fillers made the copybook empty.") } } @@ -316,7 +523,9 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { throw new RuntimeException("Can only restrict the copybook to a group element.") val newRoot = Group.root.copy(children = mutable.ArrayBuffer(stmt))(None) val schema = new BinaryPropertiesAdder().transform(newRoot) - new Copybook(schema) + val cpy = new Copybook(schema) + cpy.setVariableSizeOccursPolicy(variableSizeOccursPolicy) + cpy } /** @@ -334,6 +543,10 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { visitGroup(ast) } + private[cobrix] def setVariableSizeOccursPolicy(variableSizeOccursPolicy: VariableSizeOccursPolicy): Unit = { + this.variableSizeOccursPolicy = variableSizeOccursPolicy + } + private def getPrimitiveFieldByName(fieldName: String): Primitive = { val cachedPrimitive = cachePrimitives.get(fieldName) @@ -414,7 +627,9 @@ object Copybook { val schema1 = BinaryPropertiesAdder().transform(newRoot) val schema = ParentGroupSetter().transform(schema1) - new Copybook(schema) + val cpy = new Copybook(schema) + cpy.setVariableSizeOccursPolicy(copybooks.head.variableSizeOccursPolicy) + cpy } /** diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala index 1d4905503..9304fff61 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala @@ -28,7 +28,7 @@ import za.co.absa.cobrix.cobol.parser.exceptions.SyntaxErrorException import za.co.absa.cobrix.cobol.parser.expression.ExpressionEvaluator import za.co.absa.cobrix.cobol.parser.policies.DebugFieldsPolicy.DebugFieldsPolicy import za.co.absa.cobrix.cobol.parser.policies.StringTrimmingPolicy.StringTrimmingPolicy -import za.co.absa.cobrix.cobol.parser.policies.{CommentPolicy, DebugFieldsPolicy, FillerNamingPolicy, StringTrimmingPolicy} +import za.co.absa.cobrix.cobol.parser.policies._ import java.nio.charset.{Charset, StandardCharsets} import scala.annotation.tailrec @@ -116,6 +116,7 @@ object CopybookParser extends Logging { * resolving segment redefines. * @param fieldParentMap A segment fields parent mapping. * @param stringTrimmingPolicy Specifies if and how strings should be trimmed when parsed. + * @param variableSizeOccursPolicy Specifies the policy of OCCURS DEPENDING ON layout. * @param isDisplayAlwaysString If true, all fields having DISPLAY format will remain strings and won't be converted to numbers. * @param strictSignOverpunch If true sign overpunching is not allowed for unsigned numbers. * @param improvedNullDetection If true, string values that contain only zero bytes (0x0) will be considered null. @@ -138,6 +139,7 @@ object CopybookParser extends Logging { segmentIdRedefineMap: Map[String, String] = Map.empty, fieldParentMap: Map[String, String] = HashMap[String, String](), stringTrimmingPolicy: StringTrimmingPolicy = StringTrimmingPolicy.TrimBoth, + variableSizeOccursPolicy: VariableSizeOccursPolicy = VariableSizeOccursPolicy.MaxSize, isDisplayAlwaysString: Boolean = false, commentPolicy: CommentPolicy = CommentPolicy(), strictSignOverpunch: Boolean = true, @@ -162,6 +164,7 @@ object CopybookParser extends Logging { segmentIdRedefineMap, fieldParentMap, stringTrimmingPolicy, + variableSizeOccursPolicy, isDisplayAlwaysString, commentPolicy, strictSignOverpunch, @@ -211,6 +214,7 @@ object CopybookParser extends Logging { segmentIdRedefineMap: Map[String, String] = Map.empty, fieldParentMap: Map[String, String] = HashMap[String, String](), stringTrimmingPolicy: StringTrimmingPolicy = StringTrimmingPolicy.TrimBoth, + variableSizeOccursPolicy: VariableSizeOccursPolicy = VariableSizeOccursPolicy.MaxSize, isDisplayAlwaysString: Boolean = false, commentPolicy: CommentPolicy = CommentPolicy(), strictSignOverpunch: Boolean = true, @@ -235,6 +239,7 @@ object CopybookParser extends Logging { segmentIdRedefineMap, fieldParentMap, stringTrimmingPolicy, + variableSizeOccursPolicy, isDisplayAlwaysString, commentPolicy, strictSignOverpunch, @@ -286,6 +291,7 @@ object CopybookParser extends Logging { segmentIdRedefineMap: Map[String, String], fieldParentMap: Map[String, String], stringTrimmingPolicy: StringTrimmingPolicy, + variableSizeOccursPolicy: VariableSizeOccursPolicy, isDisplayAlwaysString: Boolean, commentPolicy: CommentPolicy, strictSignOverpunch: Boolean, @@ -346,7 +352,10 @@ object CopybookParser extends Logging { else transformedAst - new Copybook(finalAst) + val cpy = new Copybook(finalAst) + cpy.setVariableSizeOccursPolicy(variableSizeOccursPolicy) + + cpy } /** diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala index c29bc74fa..b84a27e45 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/reader/schema/CobolSchema.scala @@ -110,6 +110,7 @@ object CobolSchema { segmentIdRedefineMap, fieldParentMap, readerParameters.stringTrimmingPolicy, + readerParameters.variableSizeOccurs, readerParameters.isDisplayAlwaysString, readerParameters.commentPolicy, readerParameters.strictSignOverpunch, @@ -136,6 +137,7 @@ object CobolSchema { segmentIdRedefineMap, fieldParentMap, readerParameters.stringTrimmingPolicy, + readerParameters.variableSizeOccurs, readerParameters.isDisplayAlwaysString, readerParameters.commentPolicy, readerParameters.strictSignOverpunch, diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala new file mode 100644 index 000000000..167b14824 --- /dev/null +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala @@ -0,0 +1,247 @@ +/* + * Copyright 2018 ABSA Group Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package za.co.absa.cobrix.cobol.parser + +import org.scalatest.wordspec.AnyWordSpec +import za.co.absa.cobrix.cobol.parser.expression.ExpressionEvaluator +import za.co.absa.cobrix.cobol.parser.policies.VariableSizeOccursPolicy + +class CopybookSuite extends AnyWordSpec { + val copybookStr: String = + """ 01 RECORD. + | 05 SEGMENT-ID PIC X(1). + | 05 SEG1. + | 10 RT1 PIC X(1). + | 10 V11 PIC X(3). + | 10 V12 PIC 9(9) REDEFINES V11. + | 05 SEG2 REDEFINES SEG1. + | 10 RT2 PIC X(1). + | 10 V21 PIC X(3). + | 10 V22 PIC 9(9) REDEFINES V21. + | 05 SEG3 REDEFINES SEG1. + | 10 CNT PIC 9(1). + | 10 AR PIC X(1) OCCURS 5 TIMES + | DEPENDING ON CNT. + | 10 RT3 PIC X(1). + | 10 V31 PIC X(3). + | 10 V32 PIC 9(9) REDEFINES V31. + |""".stripMargin + + val exampleRecordSeg1Type1: Array[Byte] = Array(0xF1, 0xF1, 0xC1, 0xC2, 0xC3).map(_.toByte) + val exampleRecordSeg1Type2: Array[Byte] = Array(0xF1, 0xF2, 0xF1, 0xF2, 0xF3).map(_.toByte) + val exampleRecordSeg2Type1: Array[Byte] = Array(0xF2, 0xF1, 0xC4, 0xC5, 0xC6).map(_.toByte) + val exampleRecordSeg2Type2: Array[Byte] = Array(0xF2, 0xF2, 0xF4, 0xF5, 0xF6).map(_.toByte) + val exampleRecordSeg3Type1Shift: Array[Byte] = Array(0xF3, 0xF1, 0xC1, 0xF1, 0xC1, 0xC2, 0xC3).map(_.toByte) + val exampleRecordSeg3Type2Shift: Array[Byte] = Array(0xF3, 0xF1, 0x81, 0xF2, 0xF5, 0xF6, 0xF7).map(_.toByte) + val exampleRecordSeg3Type1Max: Array[Byte] = Array(0xF3, 0xF2, 0xC1, 0xC2, 0x00, 0x00, 0x00, 0xF1, 0xC1, 0xC2, 0xC3).map(_.toByte) + val exampleRecordSeg3Type2Max: Array[Byte] = Array(0xF3, 0xF2, 0x81, 0x82, 0x00, 0x00, 0x00, 0xF2, 0xF5, 0xF6, 0xF7).map(_.toByte) + + val segmentIdRedefineMap: Map[String, String] = Map ( + "1" -> "SEG1", + "2" -> "SEG2", + "3" -> "SEG3", + "4" -> "SEG3" + ) + + val redefineRuleExpressions: Map[String, ExpressionEvaluator] = Map[String, ExpressionEvaluator] ( + "V11" -> new ExpressionEvaluator("RT1 = '1'"), + "V12" -> new ExpressionEvaluator("RT1 = '2'"), + "V21" -> new ExpressionEvaluator("RT2 = '1'"), + "V22" -> new ExpressionEvaluator("RT2 = '2'"), + "V31" -> new ExpressionEvaluator("RT3 = '1'"), + "V32" -> new ExpressionEvaluator("RT3 = '2'") + ) + + val copybook: Copybook = CopybookParser.parse(copybookStr, + segmentIdRedefineMap = segmentIdRedefineMap, + redefineRuleExpressions = redefineRuleExpressions) + + "hasRedefineRules" should { + "return false if there are no redefine rules" in { + val copybookWithoutRedefineRules: Copybook = CopybookParser.parse(copybookStr) + assert(!copybookWithoutRedefineRules.hasRedefineRules) + } + + "return true if at least one rule is defined" in { + assert(copybook.hasRedefineRules) + } + } + + "extractExpressionVariablesFromRecord" should { + "extract variables properly for the segment 1, record type = 1" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg1Type1, Some("1")) + + assert(vars.size == 1) + assert(vars.contains("RT1")) + assert(vars("RT1") == "1") + } + + "extract variables properly for the segment 1, record type = 2" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg1Type2, Some("1")) + + assert(vars.size == 1) + assert(vars.contains("RT1")) + assert(vars("RT1") == "2") + } + + "extract variables properly for the segment 2, record type = 1" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg2Type1, Some("2")) + + assert(vars.size == 1) + assert(vars.contains("RT2")) + assert(vars("RT2") == "1") + } + + "extract variables properly for the segment 2, record type = 2" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg2Type2, Some("2")) + + assert(vars.size == 1) + assert(vars.contains("RT2")) + assert(vars("RT2") == "2") + } + + "extract variables properly for the segment 3, record type = 1, max_size" in { + copybook.setVariableSizeOccursPolicy(VariableSizeOccursPolicy.MaxSize) + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg3Type1Max, Some("3")) + + assert(vars.size == 1) + assert(vars.contains("RT3")) + assert(vars("RT3") == "1") + } + + "extract variables properly for the segment 3, record type = 2, max_size" in { + copybook.setVariableSizeOccursPolicy(VariableSizeOccursPolicy.MaxSize) + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg3Type2Max, Some("3")) + + assert(vars.size == 1) + assert(vars.contains("RT3")) + assert(vars("RT3") == "2") + } + + "extract variables properly for the segment 3, record type = 1, shifted" in { + copybook.setVariableSizeOccursPolicy(VariableSizeOccursPolicy.PadRecord) + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg3Type1Shift, Some("3")) + + assert(vars.size == 1) + assert(vars.contains("RT3")) + assert(vars("RT3") == "1") + } + + "extract variables properly for the segment 3, record type = 2, shifted" in { + copybook.setVariableSizeOccursPolicy(VariableSizeOccursPolicy.ShiftRecord) + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg3Type2Shift, Some("3")) + + assert(vars.size == 1) + assert(vars.contains("RT3")) + assert(vars("RT3") == "2") + } + } + + "isFieldEnabled" should { + "work for segment 1 record type 1" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg1Type1, Some("1")) + val isField1Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V11"), Some("1"), vars) + val isField2Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V12"), Some("1"), vars) + val isField3Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V21"), Some("1"), vars) + val isField4Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V22"), Some("1"), vars) + + assert(isField1Enabled) + assert(!isField2Enabled) + assert(!isField3Enabled) + assert(!isField4Enabled) + } + + "work for segment 1 record type 2" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg1Type2, Some("1")) + val isField1Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V11"), Some("1"), vars) + val isField2Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V12"), Some("1"), vars) + val isField3Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V21"), Some("1"), vars) + val isField4Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V22"), Some("1"), vars) + + assert(!isField1Enabled) + assert(isField2Enabled) + assert(!isField3Enabled) + assert(!isField4Enabled) + } + + "work for segment 2 record type 1" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg2Type1, Some("2")) + + val isField1Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V11"), Some("2"), vars) + val isField2Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V12"), Some("2"), vars) + val isField3Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V21"), Some("2"), vars) + val isField4Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V22"), Some("2"), vars) + + assert(!isField1Enabled) + assert(!isField2Enabled) + assert(isField3Enabled) + assert(!isField4Enabled) + } + + "work for segment 2 record type 2" in { + val vars = copybook.extractExpressionVariablesFromRecord(exampleRecordSeg2Type2, Some("2")) + + val isField1Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V11"), Some("2"), vars) + val isField2Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG1.V12"), Some("2"), vars) + val isField3Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V21"), Some("2"), vars) + val isField4Enabled = copybook.isFieldEnabled(copybook.getFieldByName("SEG2.V22"), Some("2"), vars) + + assert(!isField1Enabled) + assert(!isField2Enabled) + assert(!isField3Enabled) + assert(isField4Enabled) + } + + } + + "isPartOfSegment" should { + "work for segment 1" in { + //assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "1")) + //assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "1")) + //assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.RT2"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V21"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V22"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.RT3"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V31"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V32"), "1")) + } + "work for segment 2" in { + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "2")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "2")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "2")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2.RT2"), "2")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V21"), "2")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V22"), "2")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.RT3"), "2")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V31"), "2")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V32"), "2")) + } + "work for segment 3" in { + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "3")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "3")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "3")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.RT2"), "3")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V21"), "3")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V22"), "3")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3.RT3"), "3")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V31"), "3")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V32"), "3")) + } + } + +} diff --git a/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/SparkCobolProcessorSuite.scala b/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/SparkCobolProcessorSuite.scala index 131ee3670..55b6e7d9f 100644 --- a/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/SparkCobolProcessorSuite.scala +++ b/spark-cobol/src/test/scala/za/co/absa/cobrix/spark/cobol/SparkCobolProcessorSuite.scala @@ -21,6 +21,7 @@ import org.scalatest.wordspec.AnyWordSpec import za.co.absa.cobrix.cobol.processor.{CobolProcessingStrategy, CobolProcessorContext, SerializableRawRecordProcessor} import za.co.absa.cobrix.spark.cobol.source.base.SparkTestBase import za.co.absa.cobrix.spark.cobol.source.fixtures.{BinaryFileFixture, TextComparisonFixture} +import za.co.absa.cobrix.spark.cobol.utils.SparkUtils class SparkCobolProcessorSuite extends AnyWordSpec with SparkTestBase with BinaryFileFixture with TextComparisonFixture { private val copybook = @@ -341,6 +342,176 @@ class SparkCobolProcessorSuite extends AnyWordSpec with SparkTestBase with Binar } } + "support files with redefine rules" in { + val copybookStr: String = + """ 01 RECORD. + | 05 SEGMENT-ID PIC X(1). + | 05 SEG1. + | 10 RT1 PIC X(1). + | 10 V11 PIC X(3). + | 10 V12 PIC 9(3) REDEFINES V11. + | 05 SEG2 REDEFINES SEG1. + | 10 RT2 PIC X(1). + | 10 V21 PIC X(3). + | 10 V22 PIC 9(3) REDEFINES V21. + | 05 SEG3 REDEFINES SEG1. + | 10 CNT PIC 9(1). + | 10 AR PIC X(1) OCCURS 5 TIMES + | DEPENDING ON CNT. + | 10 RT3 PIC X(1). + | 10 V31 PIC X(3). + | 10 V32 PIC 9(3) REDEFINES V31. + |""".stripMargin + + val binData: Array[Byte] = Array( + 0x00, 0x00, 0x05, 0x00, 0xF1, 0xF1, 0xC1, 0xC2, 0xC3, // exampleRecordSeg1Type1 + 0x00, 0x00, 0x05, 0x00, 0xF1, 0xF2, 0xF1, 0xF2, 0xF3, // exampleRecordSeg1Type2 + 0x00, 0x00, 0x05, 0x00, 0xF2, 0xF1, 0xC4, 0xC5, 0xC6, // exampleRecordSeg2Type1 + 0x00, 0x00, 0x05, 0x00, 0xF2, 0xF2, 0xF4, 0xF5, 0xF6, // exampleRecordSeg2Type2 + 0x00, 0x00, 0x07, 0x00, 0xF3, 0xF1, 0xC1, 0xF1, 0xC1, 0xC2, 0xC3, // exampleRecordSeg3Type1Shift + 0x00, 0x00, 0x07, 0x00, 0xF3, 0xF1, 0x81, 0xF2, 0xF5, 0xF6, 0xF7 // exampleRecordSeg3Type2Shift + ).map(_.toByte) + + val expected = + """[ { + | "SEGMENT_ID" : "1", + | "SEG1" : { + | "RT1" : "1", + | "V11" : "aBC" + | } + |}, { + | "SEGMENT_ID" : "1", + | "SEG1" : { + | "RT1" : "2", + | "V12" : 124 + | } + |}, { + | "SEGMENT_ID" : "2", + | "SEG2" : { + | "RT2" : "1", + | "V21" : "dEF" + | } + |}, { + | "SEGMENT_ID" : "2", + | "SEG2" : { + | "RT2" : "2", + | "V22" : 457 + | } + |}, { + | "SEGMENT_ID" : "3", + | "SEG3" : { + | "CNT" : 1, + | "AR" : [ "A" ], + | "RT3" : "1", + | "V31" : "ABC" + | } + |}, { + | "SEGMENT_ID" : "3", + | "SEG3" : { + | "CNT" : 1, + | "AR" : [ "a" ], + | "RT3" : "2", + | "V32" : 567 + | } + |} ] + |""".stripMargin + withTempDirectory("spark_cobol_processor") { tempDir => + val inputPath = new Path(tempDir, "input.dat").toString + val outputPath = new Path(tempDir, "output").toString + val outputFile = new Path(outputPath, "input.dat").toString + + writeBinaryFile(inputPath, binData) + + val options = Map( + "segment_field" -> "SEGMENT_ID", + "variable_size_occurs" -> "true", + "redefine_segment_id_map:1" -> "SEG1 => 1", + "redefine_segment_id_map:2" -> "SEG2 => 2", + "redefine_segment_id_map:3" -> "SEG3 => 3", + "redefine-rule:1" -> "V11 => RT1 = '1'", + "redefine-rule:2" -> "V12 => RT1 = '2'", + "redefine-rule:3" -> "V21 => RT2 = '1'", + "redefine-rule:4" -> "V22 => RT2 = '2'", + "redefine-rule:5" -> "V31 => RT3 = '1'", + "redefine-rule:6" -> "V32 => RT3 = '2'" + ) + + SparkCobolProcessor.builder + .withCopybookContents(copybookStr) + .option("record_format", "V") + .option("is_rdw_big_endian", "false") + .options(options) + .withProcessingStrategy(CobolProcessingStrategy.ToVariableLength) + .withRecordProcessor(new SerializableRawRecordProcessor { + override def processRecord(record: Array[Byte], ctx: CobolProcessorContext): Array[Byte] = { + val segIdValue = ctx.copybook.getFieldValueByName("SEGMENT_ID", record).toString + val vars = ctx.copybook.extractExpressionVariablesFromRecord(record, Option(segIdValue)) + + segIdValue match { + case "1" => + val isV11Enabled = ctx.copybook.isFieldEnabled(ctx.copybook.getFieldByName("SEG1.V11"), Some("1"), vars) + val isV12Enabled = ctx.copybook.isFieldEnabled(ctx.copybook.getFieldByName("SEG1.V12"), Some("1"), vars) + if (isV11Enabled) { + val v11 = ctx.copybook.getFieldValueByName("SEG1.V11", record).toString + val v11Updated = s"${v11.head.toLower}${v11(1)}${v11{2}}" + ctx.copybook.setFieldValueByName("SEG1.V11", record, v11Updated) + } + if (isV12Enabled) { + val v12 = ctx.copybook.getFieldValueByName("SEG1.V12", record).asInstanceOf[Int] + val v12Updated = v12 + 1 + ctx.copybook.setFieldValueByName("SEG1.V12", record, v12Updated) + } + case "2" => + val isV21Enabled = ctx.copybook.isFieldEnabled(ctx.copybook.getFieldByName("SEG2.V21"), Some("2"), vars) + val isV22Enabled = ctx.copybook.isFieldEnabled(ctx.copybook.getFieldByName("SEG2.V22"), Some("2"), vars) + if (isV21Enabled) { + val v21 = ctx.copybook.getFieldValueByName("SEG2.V21", record).toString + val v21Updated = s"${v21.head.toLower}${v21(1)}${v21{2}}" + ctx.copybook.setFieldValueByName("SEG2.V21", record, v21Updated) + } + if (isV22Enabled) { + val v22 = ctx.copybook.getFieldValueByName("SEG2.V22", record).asInstanceOf[Int] + val v22Updated = v22 + 1 + ctx.copybook.setFieldValueByName("SEG2.V22", record, v22Updated) + } + case "3" => + val isV31Enabled = ctx.copybook.isFieldEnabled(ctx.copybook.getFieldByName("SEG3.V31"), Some("3"), vars) + val isV32Enabled = ctx.copybook.isFieldEnabled(ctx.copybook.getFieldByName("SEG3.V32"), Some("3"), vars) + if ((isV31Enabled && isV32Enabled) || (!isV31Enabled && !isV32Enabled)) + throw new IllegalArgumentException("Unexpected condition for redefine rules of segment 3. Only one should be valid") + case _ => throw new IllegalArgumentException(s"Unexpected segment id: $segIdValue") + } + + record + } + }) + .load(inputPath) + .save(outputPath) + + val outputData = readBinaryFile(outputFile) + + assert(outputData.sameElements( + Array(0, 5, 0, 0, -15, -15, -127, -62, -61, 0, 5, 0, 0, -15, -14, -15, -14, -12, 0, 5, 0, 0, + -14, -15, -124, -59, -58, 0, 5, 0, 0, -14, -14, -12, -11, -9, 0, 7, 0, 0, -13, -15, -63, + -15, -63, -62, -61, 0, 7, 0, 0, -13, -15, -127, -14, -11, -10, -9).map(_.toByte) + )) + + val actualDf = spark.read + .format("cobol") + .option("copybook_contents", copybookStr) + .option("record_format", "V") + .option("is_rdw_big_endian", "true") + .option("pedantic", "true") + .options(options) + .load(outputFile) + + val actual = SparkUtils.convertDataFrameToPrettyJSON(actualDf) + + compareText(actual, expected) + } + } + + "convert input format into an RDD without indexes" in { val expected = """-13, -14, -15""" withTempDirectory("spark_cobol_processor") { tempDir => @@ -380,7 +551,6 @@ class SparkCobolProcessorSuite extends AnyWordSpec with SparkTestBase with Binar val binData = Array(0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF1).map(_.toByte) val inputPath = new Path(tempDir, "input.dat").toString - val outputPath = new Path(tempDir, "output").toString writeBinaryFile(inputPath, binData) From 447490a1ac059718e0e4afd4ab06cbdbbc2d1d5e Mon Sep 17 00:00:00 2001 From: Ruslan Iushchenko Date: Tue, 8 Sep 2026 10:33:46 +0200 Subject: [PATCH 4/6] #871 Add `CopybookParser.parse()` overload that accepts a map of Cobrix options, deriving all parsing settings from reader parameters. --- .../cobrix/cobol/parser/CopybookParser.scala | 38 +++++++++++++--- .../copybooks/ParseCopybookFeaturesSpec.scala | 43 +++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala index 9304fff61..1c25826da 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/CopybookParser.scala @@ -29,6 +29,8 @@ import za.co.absa.cobrix.cobol.parser.expression.ExpressionEvaluator import za.co.absa.cobrix.cobol.parser.policies.DebugFieldsPolicy.DebugFieldsPolicy import za.co.absa.cobrix.cobol.parser.policies.StringTrimmingPolicy.StringTrimmingPolicy import za.co.absa.cobrix.cobol.parser.policies._ +import za.co.absa.cobrix.cobol.reader.parameters.{CobolParametersParser, Parameters} +import za.co.absa.cobrix.cobol.reader.schema.CobolSchema import java.nio.charset.{Charset, StandardCharsets} import scala.annotation.tailrec @@ -44,14 +46,36 @@ object CopybookParser extends Logging { type CopybookAST = Group - case class StatementLine(lineNumber: Int, text: String) - - case class StatementTokens(lineNumber: Int, tokens: Array[String]) - - case class CopybookLine(level: Int, name: String, lineNumber: Int, modifiers: Map[String, String]) - - case class RecordBoundary(name: String, begin: Int, end: Int) + /** + * Parses a COBOL copybook and returns the corresponding [[Copybook]] (the parsed AST with all + * properties resolved: field sizes, offsets, redefines, segment parents, fillers, etc.). + * + * This is a convenience entry point that derives all parsing settings from a map of Cobrix + * options (the same options that are passed to the `spark-cobol` data source), so callers do not + * need to provide every individual parsing parameter explicitly. + * + * Example: + * {{{ + * val copybook = CopybookParser.parse(copybookContents, Map("encoding" -> "ascii", "ascii_charset" -> "UTF-8")) + * }}} + * + * @param copyBookContents A string containing all lines of a copybook. + * @param cobolOptions A map of Cobrix reader options (option name -> option value) that define + * how the copybook and the corresponding data should be interpreted, + * e.g. encoding, code page, string trimming policy, filler handling, etc. + * Options that are not specified retain their default values. + * @throws za.co.absa.cobrix.cobol.parser.exceptions.SyntaxErrorException if the copybook cannot be parsed. + * @return A [[Copybook]] containing the AST of the parsed copybook. + */ + def parse(copyBookContents: String, cobolOptions: Map[String, String]): Copybook = { + val caseInsensitiveMap = cobolOptions.map { + case (k, v) => (k.toLowerCase, v) + } + val cobolParameters = CobolParametersParser.parse(new Parameters(caseInsensitiveMap)) + val readerParameters = CobolParametersParser.getReaderProperties(cobolParameters, None) + CobolSchema.fromReaderParameters(Seq(copyBookContents), readerParameters).copybook + } /** * Tokenizes a Cobol Copybook contents and returns the AST. diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/ParseCopybookFeaturesSpec.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/ParseCopybookFeaturesSpec.scala index 1ea45e95f..4ccfcc265 100644 --- a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/ParseCopybookFeaturesSpec.scala +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/ParseCopybookFeaturesSpec.scala @@ -197,6 +197,49 @@ class ParseCopybookFeaturesSpec extends AnyFunSuite with SimpleComparisonBase { assert(!copybook.ast.children(0).asInstanceOf[Group].children(2).isFiller) } + test("Test parse() with option passed from spark-cobol") { + val options = Map( + "drop_group_fillers" -> "false", + "drop_value_fillers" -> "false", + "pedantic" -> "true" + ) + val copybook = CopybookParser.parse(copybookFillers, options) + val layout = copybook.generateRecordLayoutPositions() + + val expectedLayout = + """-------- FIELD LEVEL/NAME --------- --ATTRIBS-- FLD START END LENGTH + | + |1 RECORD 1 1 156 156 + | 5 FILLER_P1 2 1 1 1 + | 5 COMPANY_PREFIX 3 2 4 3 + | 5 FIELD1 4 5 100 96 + | 7 FILLER_1 [] 5 5 100 96 + | 10 CHILD1 6 5 8 4 + | 10 CHILD2 7 9 12 4 + | 5 FILLER_P2 8 101 101 1 + | 5 FILLER_P3 9 102 102 1 + | 5 COMPANY_NAME r 10 103 111 9 + | 5 FILLER_2 R 11 103 111 9 + | 10 STR1 12 103 107 5 + | 10 STR2 13 108 109 2 + | 10 FILLER_P4 14 110 110 1 + | 5 ADDRESS r 15 112 141 30 + | 5 FILLER_3 R 16 112 141 30 + | 10 STR4 17 112 121 10 + | 10 FILLER_P5 18 122 141 20 + | 5 FILL_FIELD r 19 142 148 7 + | 10 FILLER_P6 20 142 146 5 + | 10 FILLER_P7 21 147 148 2 + | 5 CONTACT_PERSON R 22 142 148 7 + | 10 FIRST_NAME 23 142 147 6 + | 5 AMOUNT 24 149 156 8 + |""" + .stripMargin.replace("\r\n", "\n") + + assertEqualsMultiline(layout, expectedLayout) + assert(!copybook.ast.children(0).asInstanceOf[Group].children(2).isFiller) + } + test("Test parseSimple() renaming all fillers using previous field name policy") { val copybook = CopybookParser.parse(copybookFillers, dropGroupFillers = false, From b44aea4c986ead9e56a0c9ee1714b0fc4dbfc6da Mon Sep 17 00:00:00 2001 From: Ruslan Iushchenko Date: Wed, 9 Sep 2026 08:15:45 +0200 Subject: [PATCH 5/6] #871 Optimize `extractExpressionVariablesFromRecord` for the MaxSize OCCURS policy by extracting only fields used in REDEFINE rules via a new cached `fieldsUsedInRedefineRules` list, skipping full AST traversal. --- .../absa/cobrix/cobol/parser/Copybook.scala | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala index 452ffda43..d49abbaf9 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala @@ -29,7 +29,7 @@ import za.co.absa.cobrix.cobol.reader.parameters.WriterParameters import java.util.concurrent.ConcurrentHashMap import scala.annotation.tailrec import scala.collection.mutable -import scala.collection.mutable.ArrayBuffer +import scala.collection.mutable.{ArrayBuffer, ListBuffer} class Copybook(val ast: CopybookAST) extends Logging with Serializable { @@ -75,6 +75,7 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { def getRootSegmentIds(segmentIdRedefineMap: Map[String, String], fieldParentMap: Map[String, String]): List[String] = CopybookParser.getRootSegmentIds(segmentIdRedefineMap, fieldParentMap) + /** Returns the top-level records of the copybook. */ def getRootRecords: scala.collection.Seq[Statement] = { if (isFlatCopybook) { scala.collection.Seq(ast) @@ -83,6 +84,12 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { } } + /** + * Indicates whether the copybook contains any REDEFINE rules that need to be evaluated at parsing time. + * + * When this flag is false, no expression variables need to be extracted from records and all fields + * can be treated as enabled, which allows skipping the rule evaluation logic entirely. + */ lazy val hasRedefineRules: Boolean = { def hasAnyRules(group: Group): Boolean = { group.children.exists { @@ -93,6 +100,30 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { hasAnyRules(ast) } + /** + * A list of primitive fields of the copybook that are referenced by REDEFINE rules + * (conditional expressions that determine which redefined group applies to a record). + * + * The value is computed on first access and cached afterwards. + */ + lazy val fieldsUsedInRedefineRules: Seq[Primitive] = { + val primitives = new ListBuffer[Primitive] + + def processGroup(group: Group): Unit = { + group.children.foreach { + case g: Group => processGroup(g) + case p: Primitive => if (p.isUsedInRules) primitives += p + } + } + + if (!hasRedefineRules) { + Seq.empty + } else { + processGroup(ast) + primitives.toList + } + } + /** * Traverses the copybook AST over the given record and collects the values of all fields that * participate in expression evaluation, such as fields referenced by conditional expressions @@ -114,6 +145,25 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { segmentIdValue: Option[String] = None, startOffset: Int = 0): mutable.HashMap[String, Any] = { if (!hasRedefineRules) return mutable.HashMap.empty[String, Any] + if (variableSizeOccursPolicy == VariableSizeOccursPolicy.MaxSize) { + val variables = new mutable.HashMap[String, Any]() + + segmentIdValue match { + case Some(segId) => + fieldsUsedInRedefineRules.foreach { f => + if (isPartOfSegment(f, segId)) { + val value = Copybook.extractPrimitiveField(f, recordBytes, startOffset) + variables += (f.name -> value) + } + } + case None => + fieldsUsedInRedefineRules.foreach { f => + val value = Copybook.extractPrimitiveField(f, recordBytes, startOffset) + variables += (f.name -> value) + } + } + return variables + } val dependFields = scala.collection.mutable.HashMap.empty[String, Either[Int, String]] val variables = new mutable.HashMap[String, Any]() From 37bf04bc4d98772c8d87c93d6f2ca398d3226645 Mon Sep 17 00:00:00 2001 From: Ruslan Iushchenko Date: Thu, 10 Sep 2026 09:20:25 +0200 Subject: [PATCH 6/6] #871 Fix multiple PR suggestions. Thanks @coderabbitai for helpful tips! - Fix `isPartOfSegment` to handle group fields directly, - Match segment redefine names using the identifier transformer, - Fail merging copybooks with different variable-size OCCURS policies. --- .../absa/cobrix/cobol/parser/Copybook.scala | 16 +++++++-- .../asttransform/SegmentRedefinesMarker.scala | 6 +++- .../cobrix/cobol/parser/CopybookSuite.scala | 15 ++++++-- .../parser/copybooks/MergeCopybooksSpec.scala | 34 +++++++++++++++++++ 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala index d49abbaf9..d82d02d08 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/Copybook.scala @@ -318,7 +318,12 @@ class Copybook(val ast: CopybookAST) extends Logging with Serializable { } } - field.parent match { + val startGroup = field match { + case group: Group => Some(group) + case _ => field.parent + } + + startGroup match { case Some(p) => getSegmentRedefineGroup(p) match { case Some(segmentRedefine) => @@ -677,8 +682,15 @@ object Copybook { val schema1 = BinaryPropertiesAdder().transform(newRoot) val schema = ParentGroupSetter().transform(schema1) + val occursPolicies = copybooks.map(_.variableSizeOccursPolicy).distinct + if (occursPolicies.size > 1) { + throw new IllegalArgumentException( + "Cannot merge copybooks with different variable-size OCCURS policies." + ) + } + val cpy = new Copybook(schema) - cpy.setVariableSizeOccursPolicy(copybooks.head.variableSizeOccursPolicy) + cpy.setVariableSizeOccursPolicy(occursPolicies.head) cpy } diff --git a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala index bf27e2db9..81cc54303 100644 --- a/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala +++ b/cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/asttransform/SegmentRedefinesMarker.scala @@ -76,7 +76,11 @@ class SegmentRedefinesMarker(segmentIdRedefineMap: Map[String, String]) extends if (redefineGroupState == 1 && g.redefines.isEmpty) throw new IllegalStateException(s"The segment redefine field '${g.name}' is not a REDEFINE or redefined by another field.") - val allowedValues = segmentIdRedefineMap.filter(_._2.equalsIgnoreCase(g.name)).keys.toSeq.distinct + val allowedValues = segmentIdRedefineMap.collect { + case (segmentId, redefineName) + if transformIdentifier(redefineName).equalsIgnoreCase(g.name) => segmentId + }.toSeq.distinct + ensureSegmentRedefinesAreIneGroup(g.name, isCurrentFieldASegmentRedefine = true) foundRedefines += g.name g.withUpdatedIsSegmentRedefine(true) diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala index 167b14824..f05e59b04 100644 --- a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/CopybookSuite.scala @@ -210,34 +210,43 @@ class CopybookSuite extends AnyWordSpec { "isPartOfSegment" should { "work for segment 1" in { - //assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "1")) - //assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "1")) - //assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "1")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1"), "1")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "1")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "1")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2"), "1")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.RT2"), "1")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V21"), "1")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V22"), "1")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3"), "1")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.RT3"), "1")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V31"), "1")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V32"), "1")) } "work for segment 2" in { + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1"), "2")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "2")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "2")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "2")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2"), "2")) assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2.RT2"), "2")) assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V21"), "2")) assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V22"), "2")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3"), "2")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.RT3"), "2")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V31"), "2")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V32"), "2")) } "work for segment 3" in { + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1"), "3")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.RT1"), "3")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V11"), "3")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG1.V12"), "3")) + assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2"), "3")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.RT2"), "3")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V21"), "3")) assert(!copybook.isPartOfSegment(copybook.getFieldByName("SEG2.V22"), "3")) + assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3"), "3")) assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3.RT3"), "3")) assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V31"), "3")) assert(copybook.isPartOfSegment(copybook.getFieldByName("SEG3.V32"), "3")) diff --git a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/MergeCopybooksSpec.scala b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/MergeCopybooksSpec.scala index 686d94b3d..19c42cc56 100644 --- a/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/MergeCopybooksSpec.scala +++ b/cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/copybooks/MergeCopybooksSpec.scala @@ -18,6 +18,7 @@ package za.co.absa.cobrix.cobol.parser.copybooks import org.scalatest.funsuite.AnyFunSuite import org.slf4j.{Logger, LoggerFactory} +import za.co.absa.cobrix.cobol.parser.policies.VariableSizeOccursPolicy import za.co.absa.cobrix.cobol.parser.{Copybook, CopybookParser} import za.co.absa.cobrix.cobol.testutils.SimpleComparisonBase @@ -176,6 +177,39 @@ class MergeCopybooksSpec extends AnyFunSuite with SimpleComparisonBase { assert(exception.getMessage.contains("Cannot merge copybooks with differing root levels")) } + test("Test merge copybooks fail: differing variable size occurs policies") { + val copyBookContents1: String = + """ 01 RECORD-COPYBOOK-1. + | 05 GROUP-1. + | 06 FIELD-1 PIC X(10). + | 06 FILLER PIC X(5). + | 06 GROUP-2. + | 10 NESTED-FIELD-1 PIC 9(10). + | 10 FILLER PIC 9(5). + |""".stripMargin + val copyBookContents2: String = + """ 01 RECORD-COPYBOOK-2A. + | 05 GROUP-1. + | 06 FIELD-1 PIC X(20). + | 06 FILLER PIC X(10). + | 06 GROUP-2. + | 10 NESTED-FIELD-1 PIC 9(20). + | 10 FILLER PIC 9(10). + |""".stripMargin + + val copybook1 = CopybookParser.parseTree(copyBookContents1, variableSizeOccursPolicy = VariableSizeOccursPolicy.MaxSize) + val copybook2 = CopybookParser.parseTree(copyBookContents2, variableSizeOccursPolicy = VariableSizeOccursPolicy.ShiftRecord) + + assert(copybook1.getRecordSize == 30) + assert(copybook2.getRecordSize == 60) + + val exception = intercept[IllegalArgumentException] { + Copybook.merge(List(copybook1, copybook2)) + } + assert(exception.getMessage.contains("Cannot merge copybooks with different variable-size OCCURS policies")) + } + + test("Test merge copybooks fail: repeated identifiers") { val copyBookContents1: String = """ 01 RECORD-COPYBOOK-1.