[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1732: [SYSTEMDS-XXX] applySchema.dml() builtin

[GitBox <gi...@apache.org>]

Baunsgaard commented on code in PR #1732:
URL: https://github.com/apache/systemds/pull/1732#discussion_r1024046243


##########
src/main/java/org/apache/sysds/runtime/instructions/cp/BinaryFrameFrameCPInstruction.java:
##########
@@ -55,6 +58,16 @@ else if(getOpcode().equals("freplicate")) {
 			// Attach result frame with FrameBlock associated with output_name
 			ec.setFrameOutput(output.getName(), retBlock);
 		}
+		else if(getOpcode().equals("applySchema")) {
+			// apply frame schema from DML
+			Types.ValueType[] schema = new Types.ValueType[inBlock2.getNumColumns()];
+			for(int i=0; i<inBlock2.getNumColumns(); i++)
+				schema[i] = Types.ValueType.fromExternalString(inBlock2.get(0, i).toString());
+			FrameBlock out = new FrameBlock(schema);
+			out.copy(inBlock1);
+			out.setSchema(schema);

Review Comment:
   I would have liked it to apply it to the data inside the columns, not first copying and then setting the schema.
   this implementation just modify metadata you set, not reallocating the columns with the right data.
   
   But on the other hand this is a great start, where we can easily add the functionality inside Frame Block and Arrays.
   
   Do you want to do it ? or should i add that part?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org